-
Notifications
You must be signed in to change notification settings - Fork 114
chore(sidecar-images): fix bumping of devfiles to new sidecar tags #479
Conversation
Signed-off-by: Vitaliy Gulyy <[email protected]>
dockerfiles/base.dockerfile
Outdated
@@ -14,3 +14,4 @@ ENTRYPOINT [ "/entrypoint.sh" ] | |||
CMD ["tail", "-f", "/dev/null"] | |||
|
|||
LABEL "che.base.image" ${BASE_IMAGE} | |||
|
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.
do we need this change ?
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.
It needs to trigger the build script to rebuild all the images, then to bump devfiles to new sidecars tag and create a pull request.
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.
could we just execute the action with a parameter 'rebuild all' ?
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.
we do this when changes in base.dockerfile
, entrypoint,sh
or in install-editor-tooling.sh
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.
action build-sidecar-images-on-push.yml runs build-sidecar-images-on-push.sh which detects the changes in the last commit and decides whether it needs to rebuild only specific image or all the images.
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.
My latest commit (this fixup) will not contain anything related to the dockerfiles, and anything will be built.
To be able to build the images manually we need to add another gh-action. But do we need it?
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.
so instead of having
if [ -n "${CHANGES}" ]; then
echo -e "\nRebuild ALL images"
we check as well if there is parameter saying to rebuild anyway all the images
and in the github .yaml we provide the parameter to the build.sh script (that may be empty, default case)
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.
so instead of having
if [ -n "${CHANGES}" ]; then echo -e "\nRebuild ALL images"
we check as well if there is parameter saying to rebuild anyway all the images
ok, trying to add it.
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.
@vitaliy-guliy you don't need to add a new action.
One action can be triggered manually or when we push changes
and when we trigger it manually we can add specific parameters and in that case we can ask to rebuild everything
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.
I've updated the PR. Does it look better?
Signed-off-by: Vitaliy Gulyy <[email protected]>
Have just tested the case with sequence of actions on another repository. |
Signed-off-by: Vitaliy Gulyy <[email protected]>
Signed-off-by: Vitaliy Gulyy <[email protected]>
Signed-off-by: Vitaliy Gulyy <[email protected]>
run: ./build/workflows/build-sidecar-images-on-push.sh --push --rm --update-devfiles | ||
run: | | ||
if [[ "${{ github.event.inputs.rebuild_all }}" == "true" ]]; then | ||
./build/workflows/build-sidecar-images-on-push.sh --push --rm --update-devfiles --rebuild-all |
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.
we could have a parameter EXTRA_ARG set to empty or to --rebuild-all
to avoid to duplicate the line
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.
Added it.
Signed-off-by: Vitaliy Gulyy <[email protected]>
Signed-off-by: Vitaliy Gulyy [email protected]
What does this PR do?
Fixes creating of pull request after bumping the devfiles to new sidecar tags.
Fixup for #464
Screenshot/screencast of this PR
no screenshot
What issues does this PR fix or reference?
eclipse-che/che#19695
How to test this PR?
PR Checklist
As the author of this Pull Request I made sure that:
What issues does this PR fix or reference
andHow to test this PR
completedReviewers
Reviewers, please comment how you tested the PR when approving it.