-
Notifications
You must be signed in to change notification settings - Fork 55
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
add Kustomize 5.2.1 in Kind GHA testing #43
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,7 +13,6 @@ on: | |
env: | ||
IMG_ORG: kubeflow | ||
IMG_REPO: model-registry | ||
IMG: ${{ env.IMG_ORG }}/${{ env.IMG_REPO }} | ||
PUSH_IMAGE: false | ||
BRANCH: ${{ github.base_ref }} | ||
jobs: | ||
|
@@ -37,18 +36,24 @@ jobs: | |
uses: helm/[email protected] | ||
- name: Load Local Registry Test Image | ||
env: | ||
IMG: "${{ env.IMG }}:${{ steps.tags.outputs.tag }}" | ||
IMG: "${{ env.IMG_ORG }}/${{ env.IMG_REPO }}:${{ steps.tags.outputs.tag }}" | ||
run: | | ||
kind load docker-image -n chart-testing ${IMG} | ||
- name: Create Test Registry | ||
env: | ||
IMG: "${{ env.IMG }}:${{ steps.tags.outputs.tag }}" | ||
IMG: "${{ env.IMG_ORG }}/${{ env.IMG_REPO }}:${{ steps.tags.outputs.tag }}" | ||
run: | | ||
echo "Download kustomize 5.2.1" | ||
mkdir $GITHUB_WORKSPACE/kustomize | ||
curl -s "https://raw.githubusercontent.com/kubernetes-sigs/kustomize/master/hack/install_kustomize.sh" | bash -s "5.2.1" "$GITHUB_WORKSPACE/kustomize" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same question as other PR. Can we try to avoid using curl | bash? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried first an alternative with 471563b#diff-ee244f6eeb2d7ac8c604cdc91588b49204dea5594df11b20af816ad9f4b54467R36-R39 that is using a dedicated GHA to download kustomize, but the performance was horrible due to the fact the GHA action step does much more we don't currently need. So at this time, this seems the most vanilla and default way to override for a specific version of kustomize that what bundled in GHA automatically There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. btw, by "vanilla and default way" I mean what indicated by the manual, here: https://kubectl.docs.kubernetes.io/installation/kustomize/binaries/ |
||
PATH=$GITHUB_WORKSPACE/kustomize:$PATH | ||
echo "Display Kustomize version" | ||
kustomize version | ||
echo "Deploying Model Registry using Manifests; branch ${BRANCH}" | ||
kubectl create namespace kubeflow | ||
cd manifests/kustomize/overlays/db | ||
kustomize edit set image kubeflow/model-registry:latest $IMG | ||
kubectl apply -k . | ||
kustomize build | kubectl apply -f - | ||
- name: Wait for Test Registry Deployment | ||
run: | | ||
kubectl wait --for=condition=available -n kubeflow deployment/model-registry-db --timeout=5m | ||
|
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'm probably missing something, but what's the reason for moving the IMG env variable?
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.
env at the top level of GHA cannot depend on one-another :(
so this was the original strategy by @lampajr and I'm just putting it back in place
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.
+1 you cannot define top-level env variables from other ones, that's is only possible at
step
level