-
Notifications
You must be signed in to change notification settings - Fork 118
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
Take advantage of multi files COPY to simplify Dockerfile #219
Conversation
a9304c0
to
5377ed0
Compare
5377ed0
to
4248f43
Compare
/test-integration |
4248f43
to
f953566
Compare
/test-integration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm cancel
f953566
to
2f98035
Compare
/test-integration |
/lgtm |
2f98035
to
1dd1334
Compare
@zaneb hi! Can you please have another look? Thanks! |
ce82e6e
to
766848f
Compare
766848f
to
8961360
Compare
/test-integration |
/lgtm |
Move scripts under scripts dir and config files under config dir, then copy everything at the same time when possible. Also no need to specify ./ with COPY, the default path is always in the source dir.
8961360
to
5fac384
Compare
COPY ./dualboot.ipxe /tmp/dualboot.ipxe | ||
# TODO(dtantsur): remove scripts/runironic script when we stop supporting | ||
# running both API and conductor processes via one entry point. | ||
COPY scripts/ /bin/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be scripts/*
? This is correct syntax for rsync, but I can't find any indication in the Dockerfile docs that the COPY directives treats a source with a trailing slash differently, rsync-style.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
from COPY Docker docs:
"If is a directory, the entire contents of the directory are copied, including filesystem metadata."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And right after that:
Note The directory itself is not copied, just its contents.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You know I read that like 5 times, and each time my brain transposed the position of the not
🤦
/lgtm |
/approved |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dtantsur, elfosardo The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Bug 2011753: Ironic resumes clean before raid configuration job is actually completed
Move scripts under scripts dir and config files under config dir,
then copy everything at the same time when possible.
Also no need to specify ./ with COPY, the default path is always
inside the context of the build.