Skip to content
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

docs: re-orders deploy steps and clarifies instructions #6109

Merged

Conversation

mhrivnak
Copy link
Member

Description of the change:

Previously, if you ran the suggested command kustomize build config/default | kubectl apply -f -, that would render an incorrect manifest. That is because when the user changed the value of IMAGE_TAG_BASE and IMG in the Makefile during a previous step, that change has not yet been applied to kustomize manifests at the time of running the suggested command. So the wrong image reference ends up being rendered, and the manifests get applies to the cluster. make deploy must be run first, which updates the kustomize manifest.

This change clarifies that running kustomize build config/default is part of explaining to the reader how things work, is not a required step that should apply anything to the cluster, AND importantly should not be run until after make deploy.

Motivation for the change:

Without it, the tutorial leads the user to a broken state.

Checklist

If the pull request includes user-facing changes, extra documentation is required:

Previously, if you ran the suggested command `kustomize build
config/default | kubectl apply -f -`, that would render an incorrect
manifest. That is because when the user changed the value of
`IMAGE_TAG_BASE` and `IMG` in the Makefile during a previous step, that
change has not yet been applied to kustomize manifests at the time of
running the suggested command. So the wrong image reference ends up
being rendered, and the manifests get applies to the cluster. `make
deploy` must be run first, which updates the kustomize manifest.

This change clarifies that running `kustomize build config/default` is
part of explaining to the reader how things work, is not a required step
that should apply anything to the cluster, AND importantly should not
be run until after `make deploy`.
@openshift-ci openshift-ci bot requested review from fabianvf and jmrodri October 24, 2022 13:23
Copy link
Member

@asmacdo asmacdo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

Much improved, thanks @mhrivnak

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 24, 2022
@asmacdo asmacdo merged commit 67b8a1d into operator-framework:master Oct 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants