Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docker: Copy from the correct directory in Centos6 and Alpine dockerfiles #2761

Merged
merged 1 commit into from
Oct 6, 2022

Conversation

Haroon-Khel
Copy link
Contributor

@Haroon-Khel Haroon-Khel commented Sep 30, 2022

  • commit message has one of the standard prefixes
  • faq.md updated if appropriate
  • other documentation is changed or added (if applicable)
  • playbook changes run through VPC or QPC (if you have access)
  • VPC/QPC not applicable for this PR
  • for inventory.yml changes, bastillion/nagios/jenkins updated accordingly

ref #2760

Revert copy directory changes that I made in https://github.com/adoptium/infrastructure/pull/2751/files#diff-cdadf2592f693c4816b592c519b913084ed2de4a238be6dbf42f75464798334bL7

The changes to which level of the infra repo gets copied are obsolete in that pr since I was able to get the SHA to be passed in as a parameter. More so, these changes are causing a build failure on our alpine images in the centos7_docker_image_updater job

https://ci.adoptopenjdk.net/job/centos7_docker_image_updater/268/execution/node/63/log/
https://ci.adoptopenjdk.net/job/centos7_docker_image_updater/268/execution/node/77/log/

Step 4/7 : COPY ../. /infrastructure
COPY failed: forbidden path outside the build context: ../. ()

@Haroon-Khel Haroon-Khel changed the title do not copy top level directory docker: Copy from the correct directory in Centos6 and Alpine dockerfiles Sep 30, 2022
@Haroon-Khel Haroon-Khel requested review from aahlenst, sxa and zdtsw and removed request for aahlenst September 30, 2022 15:32
@Haroon-Khel
Copy link
Contributor Author

ref #2760

@zdtsw
Copy link
Contributor

zdtsw commented Oct 1, 2022

is passing SHA1 to ansible-playbook "git_sha=$git_sha" covering all cases? if so,

name: Get Latest git commit SHA
  shell: git rev-parse HEAD
  register: git_output
  delegate_to: localhost
  ignore_errors: yes
  when: git_sha is not defined

is not needed? otherwise, still need to run "git rev-parse HEAD" which works with "copy . /ansible" ?

@Haroon-Khel
Copy link
Contributor Author

@zdtsw The choice of passing "git_sha=$git_sha" as a parameter, which we now do with these pr workflows, and the name: Get Latest git commit SHA task should cover all cases (unless im mistaken)

otherwise, still need to run "git rev-parse HEAD" which works with "copy . /ansible" ?

From #2751 (comment), we see that if we do copy the top level directory, theres no .git folder anyway, so this ansible task will not work with the pr workflows to provide a SHA

@Haroon-Khel
Copy link
Contributor Author

@zdtsw Is there anything preventing this from being merged?

ping @sxa for review

@zdtsw
Copy link
Contributor

zdtsw commented Oct 6, 2022

@zdtsw Is there anything preventing this from being merged?

ping @sxa for review

sorry, forgot to follow this PR up.

Copy link
Member

@sxa sxa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may have missed this somewhere but is there a reason why these changes are only on Alpine and CentOS6? Do we not require the same passing in of the SHAs on CentOS7/Ubuntu16.04? (And possibly my new Ubuntu 22.04 one!)

@Haroon-Khel
Copy link
Contributor Author

@sxa Ideally all build images should have it passed in and saved in the log file, but i've been focusing on alpine and centos6 recently since theyre in the github workflows and were stopping #2751 from getting merged. I can focus on the remaining build images too, but thats for a separate pr

Copy link
Member

@sxa sxa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sxa Ideally all build images should have it passed in and saved in the log file, but i've been focusing on alpine and centos6 recently since theyre in the github workflows and were stopping #2751 from getting merged. I can focus on the remaining build images too, but thats for a separate pr

Oh so it's because 2751 only modified those two and not the others. OK, but let's aim to get everything back in sync ASAP.

@Haroon-Khel Haroon-Khel merged commit 0443672 into adoptium:master Oct 6, 2022
@Haroon-Khel Haroon-Khel deleted the docker.copy branch January 5, 2023 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants