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

[CLD-8507] Upgrade Go version, controller-runtime dependency #387

Merged
merged 20 commits into from
Nov 5, 2024

Conversation

nickmisasi
Copy link
Contributor

@nickmisasi nickmisasi commented Oct 31, 2024

Summary

Updates controller-runtime dependency to latest. This requires a Go version upgrade. The Go version upgrade required a few other package upgrades.

The controller-runtime upgrade resulted in some changes to the way the fake client handles status updates - see kubernetes-sigs/controller-runtime#2259 - adjusted tests accordingly so they'd pass.

Ticket Link

https://mattermost.atlassian.net/browse/CLD-8507

Release Note

None

@mm-cloud-bot
Copy link

@nickmisasi: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

I understand the commands that are listed here

@nickmisasi nickmisasi changed the title Upgrade Go version, controller-runtime dependency, and a bunch of oth… [CLD-8507] Upgrade Go version, controller-runtime dependency Nov 1, 2024
@mm-cloud-bot mm-cloud-bot added release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed labels Nov 1, 2024
@nickmisasi nickmisasi self-assigned this Nov 1, 2024
@nickmisasi nickmisasi added the 2: Dev Review Requires review by a developer label Nov 1, 2024
@nickmisasi nickmisasi marked this pull request as ready for review November 1, 2024 14:59
Comment on lines +83 to +84
TEMP_DIR=$(mktemp -d)
trap 'rm -rf "${TEMP_DIR}"' EXIT
Copy link
Contributor Author

Choose a reason for hiding this comment

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

--output-dir became a required param with the upgraded version - this just creates a temporary dir that's later deleted to use for that arg.

Copy link
Collaborator

@gabrieljackson gabrieljackson left a comment

Choose a reason for hiding this comment

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

Looks great! Just one small thing to update.

@@ -21,7 +21,7 @@ jobs:
fetch-depth: 0

- name: ci/setup-go
uses: actions/setup-go@6edd4406fa81c3da01a34fa6f6343087c207a568 # v3.5.0
uses: actions/setup-go@41dfa10bad2bb2ae585af6ee5bb4d7d973ad74ed # v3.5.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we get this version number updated to avoid possible future confusion? Same thing for the one below.

Copy link
Contributor

@fmartingr fmartingr left a comment

Choose a reason for hiding this comment

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

👍 At some point it would be useful to unify the Golang version from the go.mod file and just pass it along to the files that require it.

Co-authored-by: Felipe Martin <812088+fmartingr@users.noreply.github.com>
@nickmisasi nickmisasi added 3: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a developer labels Nov 5, 2024
@nickmisasi nickmisasi merged commit b39538b into master Nov 5, 2024
5 checks passed
@nickmisasi nickmisasi deleted the CLD-8507-2 branch November 5, 2024 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3: Reviews Complete All reviewers have approved the pull request release-note-none Denotes a PR that doesn't merit a release note.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants