-
Notifications
You must be signed in to change notification settings - Fork 33
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
update location of where kustomize looks for deploying example gateway #1090
update location of where kustomize looks for deploying example gateway #1090
Conversation
hack/quickstart-setup.sh
Outdated
@@ -462,7 +462,7 @@ success "Kuadrant sample configuration deployed." | |||
|
|||
# Deploy gateway | |||
info "Deploying example gateway ..." | |||
kustomize build config/dependencies/istio/gateway | kubectl apply -f - | |||
kustomize build ${KUADRANT_REPO_RAW}/config/dependencies/istio/gateway | kubectl apply -f - |
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 haven't tried this but I am always wondering why write this in this fashion, why use a pipe?
kubectl apply -k ${KUADRANT_REPO_RAW}/config/dependencie/istio/gateway
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 believe it's because it's the output of kustomize build that's needed rather than the raw directory.
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.
This I understand, the raw directory will fail if the -f
flag is used. In my example it is the -k / --kustomize
flag that is used. This requires a kustomization directory and under the hood will run kustomize build.
If you try using a file with the -k
it will fail, like how -f
fails with a directory.
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.
ah okay. I think that's what @david-martin is referring to also here -> Kuadrant/docs.kuadrant.io#161 (comment) so I will make those updates as part of this pr. 👍
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.
ack
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.
These changes are ready for review now.
090fdc3
to
afc4ede
Compare
Signed-off-by: Laura Fitzgerald <[email protected]>
afc4ede
to
5c05be8
Compare
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.
This all looks good to me and works.
@Boomatang I notice the option to merge is Rebase and Merge rather than Merge pull request, I'm blocked from doing the latter. Is it correct to do the former? |
For me I have being using the |
What
$subject so the user doesn't need to have a local copy of the repo to run the quickstart
also removing the need to have kustomize as a dependency for running the quickstart
Verification
Temporarily uninstall kustomize from your system.
Run
curl "https://raw.githubusercontent.com/laurafitzgerald/kuadrant-operator/refs/heads/remove-local-repo-depedency/hack/quickstart-setup.sh"| bash
Reinstall kustomize