Don't make unnecessary local changes in make targets

[swiatekm]

Kustomize works by applying successive overlays over resources. Currently, when we need to apply changes to our base, we do it directly in our manifests, modifying files which are checked into source control. This is annoying for local work, though doesn't really matter for CI.

This change uses a separate overlay for any changes we might want to make over our defaults. The kustomization.yaml file lives in the ignored ./dist directory, and is created and modified on demand.

I tested all the targets making use of this, and everything worked as expected, but I'd appreciate review from someone involved in writing this Makefile.

This is part of my effort to make running E2E tests locally nicer.

[pavolloffay]

I think this partly solves the problem with temporary changes applied to the checked-in files. For instance the CSV is still modified.

I would assume that if no code is changed in the repository, then the developer workflow IMG=docker.io/${USER}/opentelemetry-operator:dev-$(git rev-parse --short HEAD)-$(date +%s) make generate bundle (documented in the contributing) will not produce any changes.

IMG=docker.io/${USER}/opentelemetry-operator:dev-$(git rev-parse --short HEAD)-$(date +%s) make generate bundle 
git status                                                                                                                                    ploffay@fedora
On branch PR1794
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	modified:   bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml
ls dist                                                                                                                          130 ↵ ploffay@fedora
kustomization.yaml  opentelemetry-operator.yaml
------------------------------------------------

On the main branch the changes look like:

~/projects/open-telemetry/opentelemetry-operator (main*) » g s                                                                                                                                      ploffay@fedora
On branch main
Your branch is up to date with 'up/main'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	modified:   bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml
	modified:   config/manager/kustomization.yaml

no changes added to commit (use "git add" and/or "git commit -a")

That said, I like the improvement, but it's a small benefit for unnecessary more complicated workflow in the makefile.

[swiatekm]

Yeah, I wasn't really trying to address the bundle generation, which I don't understand as well as the kustomize parts. Maybe we can generate the bundle into a temporary folder in the same way, and only check it into source control when releasing?

I'd like to do that in a separate PR though.

[pavolloffay]

Maybe we can generate the bundle into a temporary folder in the same way, and only check it into source control when releasing?

I am not quite sure how to handle this, but I would like to land it in a single PR since its solving the same problem.

I find the checked-in bundle directory useful and that it represents the current state of the repository. The released bundles can be found in the OLM repos (see the release.md).

[swiatekm]

I find the checked-in bundle directory useful and that it represents the current state of the repository. The released bundles can be found in the OLM repos (see the release.md).

Why does the checked-in bundle exist? Is it just to be able to track changes to it? If so, we can change the current command to make release-bundle, and add a new make local-bundle which would ignore some fields. It's kind of a hack, but I don't see a better way.

Incidentally, operator-framework/operator-sdk#6285 recommends to just not keep the bundle in source control and to not run make bundle in CI.

[swiatekm]

@pavolloffay the way I see it, there are three different reasons to generate a bundle:

1. We're releasing, and we actually want to publish it
2. We want to ensure our local changes (for example in CRDs) are reflected in the checked-in bundle
3. We want to run our local changes against an OpenShift cluster, and want an actual bundle with all the local changes included.

Seems like we should have different commands for these.

1. The current bundle command could become release-bundle and continue working the same way.
2. We could have a update-bundle command for this which would make sure to ignore incidental changes to metadata
3. I think 3 would require more changes to be easy, but I'm not really that familiar with OpenShift

When we tell contributors to make bundle install run, is bundle even necessary there?

[jaronoff97]

@pavolloffay mind taking another look here as well?

[jaronoff97]

I think this LGTM. Can we also update the docs to say users should run make update-bundle if they change any manifests things?

[swiatekm]

I think this LGTM. Can we also update the docs to say users should run make update-bundle if they change any manifests things?

We have a make target called ensure-generate-is-noop which checks this and prints an error message instructing the user to run make update-bundle if needed. This runs as part of make test, so it should catch problems eariy.

I've also added a note to the contributing doc for completeness sake.

[pavolloffay]

It's been a long time since I wanted to look at this PR, sorry for too long delay. I think we can do this without increasing the complexity of the kustomize setup in the repository. We had the same issue in the tempo-oprator and it was solved without changed the kustomize. I will take a a look and submit a similar approach here as well.

[pavolloffay]

here is the alternative PR #2421

[swiatekm]

A lot of the issues this PR aimed to fix were resolved in #2421 and #2479. Closing this for now, might reevaluate in the future.