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

operator: Use latest operator image #490

Conversation

stevenhorsman
Copy link
Member

Use the latest operator image in the default config as the base manage kustomization typically has the release tag and the release kustomization also has a release tag pinned, so we want to use default for
development and testing the latest operator after discussion with @mythi @fitzthum & @wainersm

Use the latest operator image in the default config
as the base manage kustomization typically has the
release tag and the release kustomization also has a
release tag pinned, so we want to use default for
development and testing the latest operator

Signed-off-by: stevenhorsman <steven@uk.ibm.com>
@stevenhorsman stevenhorsman requested a review from a team as a code owner January 30, 2025 10:26
@mythi
Copy link
Contributor

mythi commented Jan 30, 2025

In #488 (comment) I made the comment that the bundle build is a bit confusing. I remember we agreed the approach in this PR but the alternative would be to just make the bundle creation use config/release and make config/manager to use latest tag.

@stevenhorsman
Copy link
Member Author

In #488 (comment) I made the comment that the bundle build is a bit confusing. I remember we agreed the approach in this PR but the alternative would be to just make the bundle creation use config/release and make config/manager to use latest tag.

Yep, that would also be completely valid. I think we'd need to update the bundle build process for that, but that's do-able

@mythi
Copy link
Contributor

mythi commented Jan 31, 2025

In #488 (comment) I made the comment that the bundle build is a bit confusing. I remember we agreed the approach in this PR but the alternative would be to just make the bundle creation use config/release and make config/manager to use latest tag.

Yep, that would also be completely valid. I think we'd need to update the bundle build process for that, but that's do-able

I'm OK either way. Thoughs @wainersm or others?

@wainersm
Copy link
Member

wainersm commented Feb 3, 2025

Hi @mythi !

In #488 (comment) I made the comment that the bundle build is a bit confusing. I remember we agreed the approach in this PR but the alternative would be to just make the bundle creation use config/release and make config/manager to use latest tag.

Yep, that would also be completely valid. I think we'd need to update the bundle build process for that, but that's do-able

I'm OK either way. Thoughs @wainersm or others?

At this point you know more than me and so you are in better position to take a decision, so whatever you think it's better I should sign in.

Nevertheless, I'm trying to understand why we update config/manager on first place. The bundle is built out of config/manifests which includes config/release as well as config/samples/ccruntime/default. It seems wrong to include the later, isn't it? That's why it end up having to update config/manager?

Copy link
Member

@fitzthum fitzthum left a comment

Choose a reason for hiding this comment

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

LGTM

I don't have a strong feeling here but this seems like a straightforward approach.

@mythi
Copy link
Contributor

mythi commented Feb 3, 2025

The bundle is built out of config/manifests which includes config/release as well as config/samples/ccruntime/default

@wainersm this is what the Operator SDK scaffolding gives with the difference that we moved to config/release (vs config/default). to keep "devel" and "release" paths separate and make bundle creation purely release based, the alternative is

-       cd config/manager && $(KUSTOMIZE) edit set image controller=$(IMG)
+       cd config/release && $(KUSTOMIZE) edit set image controller=$(IMG)

with the one-time fix to drop newTag from config/manager/kustomization.yaml

Copy link
Member

@wainersm wainersm left a comment

Choose a reason for hiding this comment

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

I suggest to merge this PR as is; @mythi can send an improvement on top of it if he wants to.

@stevenhorsman
Copy link
Member Author

Ok, based on the approvals and discussion let's get this in and re-allow the installation of latest. If we want to re-work this later, then I'm happy to help.

@stevenhorsman stevenhorsman merged commit 78f616c into confidential-containers:main Feb 6, 2025
23 of 24 checks passed
@stevenhorsman stevenhorsman deleted the update-default-config-operator-image branch February 6, 2025 10:30
@mythi
Copy link
Contributor

mythi commented Feb 6, 2025

Ok, based on the approvals and discussion let's get this in and re-allow the installation of latest. If we want to re-work this later, then I'm happy to help.

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants