bundle: createdAt field is always updated when make bundle is run

[zalsader]

Bug Report

Running make bundle always updates the createdAt field even if no other updates in the bundle are present. I was hoping that createdAt would not change unless there are other changes in the bundle. I understand that this is an expected behaviour based on #6136, but it is annoying when taking git and github into account.

I plan to create a github workflow that checks if make bundle has been run, and always having one line change is not ideal. The other place where this could be an issue is when collaborating on a project. The changes in the bundle will have to be actively ignored before commiting and pushing. The git pull operation might complain that I have local changes and prevent me from pulling until they are discarded.

I understand that there are ways around it, it is just not ideal to have to deal that.

What did you do?

Run make bundle, commit all the changes.
Run make bundle again.

What did you expect to see?

I expected to see no changes in git

What did you see instead? Under which circumstances?

The line for metadata.annotations.createdAt in bundle/manifests/<name>.clusterserviceversion.yaml is changed even though there are no other changes in the bundle.

Environment

Operator type:

/language go

Kubernetes cluster type:

minikube

$ operator-sdk version

operator-sdk version: "v1.26.0", commit: "cbeec475e4612e19f1047ff7014342afe93f60d2", kubernetes version: "1.25.0", go version: "go1.19.3", GOOS: "linux", GOARCH: "amd64"

$ go version (if language is Go)

go version go1.19.4 linux/amd64

$ kubectl version

WARNING: This version information is deprecated and will be replaced with the output from kubectl version --short.  Use --output=yaml|json to get the full version.
Client Version: version.Info{Major:"1", Minor:"26", GitVersion:"v1.26.0", GitCommit:"b46a3f887ca979b1a5d14fd39cb1af43e7e5d12d", GitTreeState:"clean", BuildDate:"2022-12-08T19:58:30Z", GoVersion:"go1.19.4", Compiler:"gc", Platform:"linux/amd64"}
Kustomize Version: v4.5.7
Server Version: version.Info{Major:"1", Minor:"25", GitVersion:"v1.25.3", GitCommit:"434bfd82814af038ad94d62ebe59b133fcb50506", GitTreeState:"clean", BuildDate:"2022-10-12T10:49:09Z", GoVersion:"go1.19.2", Compiler:"gc", Platform:"linux/amd64"}

Possible Solution

Surround this with an if statement that checks if anything else has changed.

Additional context

[scotRinga-MayaDog]

ꓝꓲꓠꓠꓲꓢꓧ

[zalsader]

Workaround:
Create this script and run it inside make bundle

#!/bin/bash
git diff --quiet -I'^    createdAt: ' bundle
if ((! $?)) ; then
    git checkout bundle
fi

[varshaprasad96]

@zalsader As you mentioned, this was an intended behaviour. From the production pipeline's perspective, we have a backend tool that looks at all the bundles and collects data on them which includes the created-At field. If the customer runs make bundle again and deploys on the cluster, even though there may not be particularly any changes in the bundle content, we track this to be an intentional behaviour considering that the customer has taken action.

I would be open if the many authors would prefer to not update this field if there is no change in bundle content. We could change the default behaviour, but I would still insist that an option remains retaining the current process of updating the field with every make bundle.

[Bhargav-InfraCloud]

+1 @zalsader, just adding my thoughts.
I haven't come across this issue, but I think createdAt should only be updated when "there is an update". I didn't get the background of #6136, on why the team at KubeCon wanted to update it every time. But I wish to understand.

[everettraven]

We discussed this issue during the community meeting yesterday (02/06/2023) and there was some agreement that the bundles should be considered and treated as release artifacts, meaning they should not be updated as part of a regular pull request but rather during the release process only (whether that is in CI or there is some manual process).

There was agreement that we need to update the documentation around this. There was also some discussion around ways to enable an author to not update that field if there are no changes in the bundle, but I don't recall there being a consensus.

I am curious, in what way are you using make bundle and running into this problem? Is it a step that must be run before a PR or is it run in CI as a check against a PR?

[varshaprasad96]

+1. Bundles are considered "release artifacts". The use of make bundle is to enable authors to package their relevant operator manifests into a format that is accepted by OLM and publish it. An iteration of make bundle indicates that I have one set of release artefacts created at that particular point in time, whether it had changes from the previous version or not is not relevant.

There was agreement that we need to update the documentation around this. There was also some discussion around ways to enable an author to not update that field if there are no changes in the bundle, but I don't recall there being a consensus.

If we would like to enable authors to update it, we shouldn't be just allowing them to do so with a flag at all times. The approach should be to check if the --overwrite flag has been set for CSV generation. If yes, then createdAt should be updated no matter what, if no then we could possibly give an option to the author to update it or not. As mentioned by Bryce, this is a separate feature, which needs to be discussed before implementing it instead of changing the current default behaviour as is.

[zalsader]

There was agreement that we need to update the documentation around this. There was also some discussion around ways to enable an author to not update that field if there are no changes in the bundle, but I don't recall there being a consensus.

I am curious, in what way are you using make bundle and running into this problem? Is it a step that must be run before a PR or is it run in CI as a check against a PR?

I was planning on running it as part of a CI check that would warn you if make bundle would have generated new changes (other than createdAt) to the bundle, asking you to run it and commit changes in case it was forgotten. An update to the documentation on when it should be rerun would be greatly appreciated.

[frzifus]

Thats what the Jaeger-Operator, tempo-operator and opentelemetry-operator does.

[zalsader]

Thats what the Jaeger-Operator, tempo-operator and opentelemetry-operator does.

I looked at the code there. These operators use an older version of the operator-sdk which did not include the updated createdAt. It was helpful to look at them, so thank you.

[frzifus]

Right, the createdAt did break it. 🦝

[everettraven]

I was planning on running it as part of a CI check that would warn you if make bundle would have generated new changes (other than createdAt) to the bundle, asking you to run it and commit changes in case it was forgotten. An update to the documentation on when it should be rerun would be greatly appreciated.

This makes sense, but I would personally recommend that instead of checking the bundle for changes in every PR you instead only check for uncommitted changes in the config/ directory after running make manifests as all the manifests in the config/ directory are run through kustomize and then the output is piped into the operator-sdk generate bundle command to package all of that into a bundle. If there are changes in the config/ directory there is likely to be changes to the generated bundle.

I definitely agree that we need to update the documentation in this regard, which we are keeping this issue open to track.

[frzifus]

@everettraven so you recommend to remove the bundle folder from the version control?

[kaovilai]

Making an update here
konveyor/operator#216

This repo prior to my PR edits bundle/ directly by hand. After the PR we would want to CI this to make sure config/ remains up to date if someone were to change bundle/ directly or vice versa.

Thus createdAt here would break CI if one were to be implemented without @zalsader 's workaround

[kaovilai]

Another makefile target to workaround.

.PHONY: bundle-ignore-createdAt
bundle-ignore-createdAt:
	git diff --quiet -I'^    createdAt: ' bundle && git checkout bundle || true

[kaovilai]

2. We can define a constant. As part of the testdata generation process we replace the createdAt value in the generated CSV with this constant. This should prevent the git diff check from failing solely because of the createdAt value.

Number 2 probably makes more sense then. I assume the constant and code changes for the testdata generation should be under the hack/ directory?

from PR
This is indeed a hack. A constant only works for test data. For a real operators repo, we would want the createdAt updated when there are changes in config/. It should be possible to simply in CI check if config/ has become out of sync with bundle/ by running make bundle in CI and diffing the bundle without hacky workarounds noted in this issue.

The only other explanation that would make sense for not honoring this issue is if the suggestion is to add bundle/ to .gitignore which further hides the resulting CSV from version control.

[redhatrises]

A couple of things/comments:

1. Agree that make bundle shouldn't be used at all as part of CI as any number of changes can update the bundle content, e.g. the project uses operator-sdk 0.9.0 and a contributor uses operator-sdk 1.2.0 and then runs make bundle. Now, your bundle is not only different, but the metadata is different from what the project uses and probably isn't correct anymore. Since make bundle is release metadata, changes should be expected.
2. Personally, I think it is not accurate to make the statement that "only changes in config/" should update the createdAt field in a release bundle because there can be an update/release of an operator that doesn't even touch or have anything to do with config/. The reason is that the createdAt field is more than just about test data. From what I understand, it is when the release artifacts/bundle was created aka make bundle. createdAt - A rough (to the day) timestamp of when the operator image was created per https://redhat-connect.gitbook.io/certified-operator-guide/ocp-deployment/operator-metadata/creating-the-csv#fields-to-add-under-metadata.annotations.
3. More importantly, users and customers will look at the version of the operator and createdAt field and correlate them which is expected. So, for example, if operator version 1.0.0 was released on 2023-07-30, and your OLM bundle still stays 2021-07-30 from its initial release or 2023-01-30 from a 0.9.0 release, creates issues for users and customers as they view your metadata as out-of-date.

So, agree that there should be documentation updates. Also, I think it makes sense to have a updatedAt field which would provide a datetime of when your operator was last updated. Then, createdAt would become a field for when your operator was initially created.

[kaovilai]

Adding updatedAt does not resolve the problems outlined in this issue.

[kaovilai]

I think it is not accurate to make the statement that "only changes in config/" should update the createdAt

Which is why this is a flag for CI only. It is so the CI do not fail. You can still intentionally commit createdAt updates.

[kaovilai]

the project uses operator-sdk 0.9.0 and a contributor uses operator-sdk 1.2.0

This is solved by using a consistent operator-sdk version for all contributors

OPERATOR_SDK = $(shell pwd)/bin/operator-sdk
operator-sdk:
	# Download operator-sdk locally if does not exist
	if [ ! -f $(OPERATOR_SDK) ]; then \
		mkdir -p bin ;\
		curl -Lo $(OPERATOR_SDK) https://github.com/operator-framework/operator-sdk/releases/download/v1.23.0/operator-sdk_$(shell go env GOOS)_$(shell go env GOARCH) ; \
		chmod +x $(OPERATOR_SDK); \
	fi

.PHONY: bundle
bundle: manifests kustomize operator-sdk ## Generate bundle manifests and metadata, then validate generated files.
	$(OPERATOR_SDK) generate kustomize manifests -q
	cd config/manager && $(KUSTOMIZE) edit set image controller=$(IMG)
	$(KUSTOMIZE) build config/manifests | $(OPERATOR_SDK) generate bundle -q --extra-service-accounts "velero" --overwrite --version $(VERSION) $(BUNDLE_METADATA_OPTS)
	@make nullables
	# Copy updated bundle.Dockerfile to CI's Dockerfile.bundle
	# TODO: update CI to use generated one
	cp bundle.Dockerfile build/Dockerfile.bundle
	$(OPERATOR_SDK) bundle validate ./bundle

[kaovilai]

users and customers will look at the version of the operator and createdAt field

Only for releases. For development branches it is important to not update createdAt just for fun. Hence why there is CI that can check for this.

[kaovilai]

My PR solves this where it will prevent git diff --exit-code failures in CI if the only thing make bundle results during CI run is createdAt change.

It will not fail CI if createdAt was the only change made if its from the user PR and not CI.

[kaovilai]

the project uses operator-sdk 0.9.0 and a contributor uses operator-sdk 1.2.0

I can't see how allowing this would be a desirable way to contribute to a repo.

[kaovilai]

There have been other issues that will be hard to debug if you allow multiple operator-sdk versions to be used.

Such as relatedImages issue here #5763
which can cause some users to fail mirroring images, and some others don't experience an issue.

The whole repo should be using the same version together.

[kaovilai]

I disagree with the notion that bundle is only for release artifacts when in practice, people have been making changes to bundle without making a release.
https://github.com/konveyor/tackle2-operator/pull/217/files

[openshift-bot]

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

[kaovilai]

/lifecycle frozen