feat: validate kustomize build of tasks and pipelines #1792
Conversation
tisutisu
force-pushed
the
kustomize-build
branch
from
fdbe145 to
3033272
Compare
tisutisu
force-pushed
the
kustomize-build
branch
3 times, most recently
from
29e718a to
fe32af3
Compare
hack/build-manifests.sh
Outdated
| cd "$SCRIPT_DIR/.." | ||
| task_dirs=$(find task -maxdepth 4 \( -name 'kustomization.yaml' -o -name 'kustomization.yml' \) -exec dirname {} \;) | ||
| pipeline_dirs=$(find pipelines -maxdepth 3 \( -name 'kustomization.yaml' -o -name 'kustomization.yml' \) -exec dirname {} \;) | ||
| local out=auto-generated |
There was a problem hiding this comment.
Instead of prepending the auto-generated/ prefix, can we put the generated files into the original directory? e.g. task/build-image-index/0.1/build-image-index.yaml?
I think that would automatically make the build-and-push.sh script notice that the task yaml already exists and it's not necessary to run kustomize again
There was a problem hiding this comment.
In that case, we have to also update the build-and-push.sh script and don't run the oc kustomize cmds again during build-and-push.
There was a problem hiding this comment.
looks like we don't need to modify the build-and-push script, if we commit the tasks and pipelines to the same directory, made the changes now.
tisutisu
force-pushed
the
kustomize-build
branch
from
fe32af3 to
c9884f3
Compare
tisutisu
requested review from
ligangty,
yma96,
amisstea,
avi-biton,
gbenhaim,
yftacherzog,
Omeramsc and
a team
as code owners
There was a problem hiding this comment.
This task is mirrored from another repo, so changes made directly will eventually be overwritten by changes made there.
There was a problem hiding this comment.
Will ignore building manifest for this task then
There was a problem hiding this comment.
This task is mirrored from another repo, so changes made directly will eventually be overwritten by changes made there.
There was a problem hiding this comment.
same for this task as well
There was a problem hiding this comment.
This task is mirrored from another repo, so changes made directly will eventually be overwritten by changes made there.
There was a problem hiding this comment.
same for this task as well
tisutisu
force-pushed
the
kustomize-build
branch
2 times, most recently
from
55f6f38 to
04430c6
Compare
tisutisu
force-pushed
the
kustomize-build
branch
from
04430c6 to
fb458d7
Compare
|
|
||
| cd "$SCRIPT_DIR/.." | ||
| task_dirs=$(find task -maxdepth 4 \( -name 'kustomization.yaml' -o -name 'kustomization.yml' \) -exec dirname {} \;) | ||
| pipeline_dirs=$(find pipelines -mindepth 2 \( -name 'kustomization.yaml' -o -name 'kustomization.yml' \) -exec dirname {} \;) |
There was a problem hiding this comment.
The task directory structure is fixed. Can you keep consistency with what build-and-push.sh does to access the tasks and pipelines directories?
There was a problem hiding this comment.
in the build-manifests.sh script, we are trying to find the task which has kustomization.yaml inside it. while in the build-and-push.sh we are building bundle and pushing for all the tasks, there is some difference. Do you see any problem in the query or want me to write the query to find the tasks in different way?
| echo "Please run ./hack/build-manifests.sh and update your PR" >&2; | ||
| exit 1; | ||
| } | ||
| fi |
There was a problem hiding this comment.
Looks like ./hack/build-manifests.sh becomes a mandatory part of the development workflow when working with tasks and pipelines. Can you describe this? Probably inside the build-manifests.sh script itself at least.
There was a problem hiding this comment.
Described it in the README change here. Do you think it is sufficient or need to be added to the build-manifests.sh script as well?
tisutisu
force-pushed
the
kustomize-build
branch
from
fb458d7 to
7aaed3c
Compare
|
/retest |
|
/retest |
This PR adds a CI check to validate kustomize build is successful and also checks that PR authors are committing built manifests files of task/pipeline change as well with their task/pipeline changes.
Part of story: https://issues.redhat.com/browse/STONEBLD-3042