USHIFT-2372: Correct the GROUP path assignment when calling build_images.sh with -t. #3011
Conversation
openshift-ci
bot
added
the
approved
|
/lgmt @jogeo , please attach this PR to a JIRA issue you're working on, or create a separate bug for tracking. |
openshift-ci
bot
added
the
do-not-merge/hold
|
/lgtm |
|
There is a secondary issue once the group path is set correctly. When -t is passed only one template is built but the code to build the image installer loops over all the files in the directory that have the .image-installer suffix. I'll keep the /hold until a fix for this is added as well. |
test/bin/build_images.sh
Outdated
| if [ -n "${template_arg}" ]; then | ||
| image_installer_list="${template_arg%.toml}".image-installer | ||
| fi | ||
| for image_installer in "${image_installer_list}"; do |
There was a problem hiding this comment.
Can we not alter the for loop pattern? There are edge cases on how wildcards are interpreted and expanded when passed through variables.
As an alternative, inside the for loop, please just check if a template_arg was given and compare the file names. If names do not match, use continue to skip to the next loop iteration.
|
/retest |
|
/retest |
jogeo
changed the title
|
@jogeo: This pull request references USHIFT-2372 which is a valid jira issue. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci-robot
added
the
jira/valid-reference
| @@ -362,6 +362,15 @@ do_group() { | |||
|
|
|||
| if ${BUILD_INSTALLER} && ! ${COMPOSER_DRY_RUN}; then | |||
| for image_installer in "${groupdir}"/*.image-installer; do | |||
| # If a template arg was given, only build the image installer for the | |||
There was a problem hiding this comment.
I'm curious - can you give me a command that has wrong behavior without the fix? I think I noticed some unnecessary installer builds, but I can't recall the exact scenario.
There was a problem hiding this comment.
test/bin/build_images.sh -E -i -f -t test/image-blueprints/layer1-base/group1/rhel93.toml
There was a problem hiding this comment.
Argh, I get it now - this one doesn't show up until you fix the primary one...
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dhellmann, ggiguash, jogeo, pmtk 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 |
|
/unhold |
openshift-ci
bot
removed
the
do-not-merge/hold
|
@jogeo: all tests passed! Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
/cherry-pick release-4.15 |
|
@ggiguash: new pull request created: #3202 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Corrects the GROUP variable value when build_images.sh is called with the -t option.