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

bump operator-sdk version to v1.36.1 #47

Merged
merged 1 commit into from
Oct 11, 2024

Conversation

lmilleri
Copy link
Member

@lmilleri lmilleri commented Oct 3, 2024

@lmilleri lmilleri requested a review from a team as a code owner October 3, 2024 12:26
@lmilleri
Copy link
Member Author

lmilleri commented Oct 4, 2024

@bpradipt PTAL

@bpradipt
Copy link
Member

bpradipt commented Oct 8, 2024

@lmilleri are there any golang module updates that needs to be done based on https://github.com/operator-framework/operator-sdk/blob/v1.36.1/go.mod ?

@lmilleri
Copy link
Member Author

lmilleri commented Oct 8, 2024

@lmilleri are there any golang module updates that needs to be done based on https://github.com/operator-framework/operator-sdk/blob/v1.36.1/go.mod ?

done

go.mod Outdated
@@ -1,15 +1,17 @@
module github.com/confidential-containers/trustee-operator

go 1.21
go 1.22
Copy link
Member

Choose a reason for hiding this comment

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

You'll need to use go 1.21 in your dev env to make this update.
The 1.36.1 sdk is still on 1.21. Although not strictly needed, but still it's good to use the same golang version as used with the sdk.

Copy link
Member Author

Choose a reason for hiding this comment

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

ok, will revert back to go 1.21

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure I should revert to go 1.21 though. When running go mod tidy the golang version is updated automatically to go 1.22.
This is because of the following dep:

go mod tidy -go=1.21 -compat=1.21
go: github.com/google/[email protected] requires [email protected], but 1.21 is requested

Copy link
Member

Choose a reason for hiding this comment

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

Hmm and do you know what is using pprof ? You will need to downgrade pprof otherwise.

Copy link
Member Author

Choose a reason for hiding this comment

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

pprof comes as an indirect dependency. The weird thing is that all the direct deps are aligned to https://github.com/operator-framework/operator-sdk/blob/v1.36.1/go.mod

Copy link
Member

Choose a reason for hiding this comment

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

Ok, one trick I use is to explicitly download a specific version of the offending dep. For example using the commit
before fa2c70bbbfe53334fe0fd4c80336583970707379 (https://github.com/google/pprof/commits/main/) should fix it imho.
But if you hit additional issues, then we'll have to stick to 1.22. Build and test should help uncover any potential issues.

Copy link
Member Author

Choose a reason for hiding this comment

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

It seems the trick works ok, Thank you

Copy link
Contributor

Choose a reason for hiding this comment

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

Go 1.21 is no longer maintained so it'd good to bump this to go 1.22.0 (note the new syntax that says "at least Go 1.22.0 toolchain is required"

@lmilleri lmilleri force-pushed the operator-sdk-bump branch 2 times, most recently from 4ef731c to 67e2e93 Compare October 10, 2024 11:50
Copy link
Member

@bpradipt bpradipt left a comment

Choose a reason for hiding this comment

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

/lgtm

@lmilleri lmilleri merged commit 3331769 into confidential-containers:main Oct 11, 2024
3 checks passed
@mythi
Copy link
Contributor

mythi commented Oct 14, 2024

We should gradually start moving away from the strict operator-sdk dependency. There's a deprecation announcement that states that controllers are free to move on after the initial scaffolding. What this means is here we can bump to more recent k8s/controller tools versions etc.

For trustee-operator, I'd suggest we add some new tests similar to what CC Operator added: test envtest against k8s vX, vX-1, and vX-2 control planes.

@bpradipt
Copy link
Member

We should gradually start moving away from the strict operator-sdk dependency. There's a deprecation announcement that states that controllers are free to move on after the initial scaffolding. What this means is here we can bump to more recent k8s/controller tools versions etc.

My understanding is that the deprecation doesn't apply to usage of operator-sdk project and applies to only OpenShift shipped cli. We are not using it anyway for this project.

Ref: section titled: "What does this mean for the Operator SDK CNCF project?"

For trustee-operator, I'd suggest we add some new tests similar to what CC Operator added: test envtest against k8s vX, vX-1, and vX-2 control planes.

Agree..

@mythi
Copy link
Contributor

mythi commented Oct 14, 2024

We are not using it anyway for this project.

I see. This PR gave me the impression that we have a dependency to operator-sdk and that we follow versions dictated by what's in Openshift. To me, the gist of the announcements is that there's less coupling going forward.

lmilleri added a commit to lmilleri/trustee-operator that referenced this pull request Dec 3, 2024
lmilleri added a commit to lmilleri/trustee-operator that referenced this pull request Dec 3, 2024
bpradipt pushed a commit to bpradipt/trustee-operator that referenced this pull request Dec 11, 2024
…references/main

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

Successfully merging this pull request may close these issues.

3 participants