Upgrade to Go 1.18 and to operator-sdk 1.23 (fixes #681) #694

olim7t · 2022-09-21T17:43:50Z

What this PR does:
Update to Go 1.18, Kubernetes 1.24, Operator SDK 1.23, controller-gen 0.9.2, Kustomize 4.5.7, controller-runtime 0.12.2.

Which issue(s) this PR fixes:
Fixes #681

Checklist

Changes manually tested
Automated Tests added/updated
Documentation added/updated
CHANGELOG.md updated (not required for documentation PRs)
CLA Signed: DataStax CLA

olim7t

I've kept separate commits so far for clarity, but I will squash everything together when merging.

olim7t · 2022-09-21T17:46:20Z

go.mod

 	github.com/bombsimon/logrusr/v2 v2.0.1
-	github.com/cespare/xxhash/v2 v2.1.2 // indirect


I moved all indirect dependencies to the second require block below.

While this may look prettier, sadly it's pretty futile. Next time someone runs go mod tidy , it will mess things up. Or go get -d.

Yeah, maybe we can just clean things up manually from time to time...

olim7t · 2022-09-21T17:46:50Z

go.mod

-)
-
-require (
-	github.com/apache/tinkerpop/gremlin-go v0.0.0-20220530191148-29272fa563ec


Not sure why there was a dedicated require block for this, I moved it to the first one above.

olim7t · 2022-09-21T17:51:38Z

go.mod

 	sigs.k8s.io/yaml v1.3.0
 )

 require (
-	cloud.google.com/go v0.93.3 // indirect
+	cloud.google.com/go v0.81.0 // indirect


My assumption is that indirect dependencies are managed by go mod, not edited by us manually.
So after updating direct dependencies, I deleted all the indirect ones and re-ran go mod tidy to restart from a clean slate.
But this results in a few lower versions like this one, I'm not sure why.

I can revisit this if this wasn't the correct approach.

The go mod commands are not "stable" in a sense that they would generate same output each time. I'm assuming they use map[] underneath for dependency graphs and resolve, in which case the iteration order is random and as such can generate different output.

So do you think the discrepancy matters here? It looks like if the version is higher than what tidy would suggest, it's not modified. I can go back to the versions from main and re-run it.

I can go back to the versions from main and re-run it.

If I do that I get those differences:

- cloud.google.com/go v0.81.0 // indirect + cloud.google.com/go v0.93.3 // indirect - github.com/Azure/go-autorest/autorest v0.11.18 // indirect - github.com/Azure/go-autorest/autorest/adal v0.9.13 // indirect + github.com/Azure/go-autorest/autorest v0.11.20 // indirect + github.com/Azure/go-autorest/autorest/adal v0.9.15 // indirect - github.com/form3tech-oss/jwt-go v3.2.3+incompatible // indirect + github.com/golang-jwt/jwt/v4 v4.0.0 // indirect + github.com/golang/snappy v0.0.4 // indirect - go.uber.org/atomic v1.7.0 // indirect + go.uber.org/atomic v1.9.0 // indirect

I don't think it matters which one it picks as long as it compiles afterwards. Clearly there's two dependencies which have different version of certain module, so picking either one could break the other.

codecov-commenter · 2022-09-21T17:54:51Z

Codecov Report

Merging #694 (cbd3745) into main (98e4f4f) will increase coverage by 0.06%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main     #694      +/-   ##
==========================================
+ Coverage   52.88%   52.94%   +0.06%     
==========================================
  Files          80       80              
  Lines        8228     8231       +3     
==========================================
+ Hits         4351     4358       +7     
+ Misses       3426     3424       -2     
+ Partials      451      449       -2

Impacted Files	Coverage Δ
pkg/medusa/requests.go	`0.00% <0.00%> (ø)`
pkg/utils/hash.go	`0.00% <0.00%> (ø)`
controllers/stargate/stargate_controller.go	`47.23% <0.00%> (+1.16%)`	⬆️
controllers/k8ssandra/stargate.go	`65.32% <0.00%> (+2.41%)`	⬆️

burmanm · 2022-09-22T14:16:51Z

CHANGELOG/CHANGELOG-1.2.md

+## V1.2.2
+
+* [ENHANCEMENT] [#681](https://github.com/k8ssandra/k8ssandra-operator/issues/681) Update to Go 1.18, Kubernetes 1.24,
+  Operator SDK 1.23, controller-gen 0.9.2, Kustomize 4.5.7, controller-runtime 0.12.2


Nit, but lets keep these one line (instead of extra forced line changes). Helps rendering.

burmanm · 2022-09-22T14:18:45Z

docs/developer/README.md

@@ -1,5 +1,5 @@
 # Requirements
-* Go >= 1.17
+* Go >= 1.18
 * kubectl >= 1.17


These other requirements are wrong since we just updated our tools. We don't know if things work with kubectl 1.17 (well, they won't) since we're on 1.24. And same for kustomize 4.0.5, which I don't think works.

Right, I didn't give this the attention it deserved (because I have more recent versions installed).

I will set:

kubectl to 1.23 (based on the version skew policy docs)

Kustomize to 4.5.7. Maybe older versions work, but I don't think trying them is worth the time. Also it's the current version installed by package managers (brew, etc.)

Kind to 0.15.0, for the same reasons.

I'll also update default_kind_node_version in setup-kind-multicluster.sh.

burmanm · 2022-09-22T14:22:58Z

Makefile

-controller-gen: ## Download controller-gen locally if necessary.
-	$(call go-get-tool,$(CONTROLLER_GEN),sigs.k8s.io/controller-tools/cmd/controller-gen@v0.8.0)
+KUSTOMIZE_INSTALL_SCRIPT ?= "https://raw.githubusercontent.com/kubernetes-sigs/kustomize/master/hack/install_kustomize.sh"
+.PHONY: kustomize


.PHONY should be in every Makefile target or none I guess. Now the behavior changes per target. I prefer every target, since we're not dealing with files (other than the $(LOCALBIN) target)

Yeah this was already inconsistent before it seems.
I can do it here, or on a separate PR if we don't want to mix the changes.

Separate PR is fine.

burmanm · 2022-09-22T14:30:33Z

Makefile

@@ -38,6 +38,17 @@ IMAGE_TAG_BASE ?= k8ssandra/k8ssandra-operator
 # You can use it as an arg. (E.g make bundle-build BUNDLE_IMG=<some-registry>/<project-name-bundle>:<tag>)
 BUNDLE_IMG ?= $(IMAGE_TAG_BASE)-bundle:v$(VERSION)

+# BUNDLE_GEN_FLAGS are the flags passed to the operator-sdk generate bundle command
+BUNDLE_GEN_FLAGS ?= -q --overwrite --version $(VERSION) $(BUNDLE_METADATA_OPTS)


This code isn't used anywhere.

Right, I need to reference the new variable when calling generate bundle in the bundle target.
Btw this also needs to be done in cass-operator.

Yep, though it's not high severity (k8ssandra-operator does not use bundles and cass-operator uses separate script when digests are used)

burmanm · 2022-09-22T14:30:45Z

Makefile

+# To enable set flag to true
+USE_IMAGE_DIGESTS ?= false
+ifeq ($(USE_IMAGE_DIGESTS), true)
+	BUNDLE_GEN_FLAGS += --use-image-digests


Same here, this code isn't used anywhere.

burmanm

kuttl wasn't updated, the first Go 1.18 version is 0.12.1, but 0.13.0 is maybe better.

burmanm · 2022-09-22T20:30:58Z

Why did you revert kind image? Now our envtest is 1.24.2, but we run manually on 1.22? That could lead to interesting cases.

olim7t · 2022-09-22T20:51:45Z

All the e2e tests are failing with the 1.24 image. I'm going to debug locally.

olim7t · 2022-09-23T19:28:07Z

config/rbac/service_account_token.yaml

+  name: k8ssandra-operator-token
+  annotations:
+    kubernetes.io/service-account.name: "k8ssandra-operator"
+type: kubernetes.io/service-account-token


This solves the issue with Kind 1.24+ images. From the Kubernetes 1.24 upgrade notes:

The LegacyServiceAccountTokenNoAutoGeneration feature gate is beta, and enabled by default. When enabled, Secret API objects containing service account tokens are no longer auto-generated for every ServiceAccount. Use the TokenRequest API to acquire service account tokens, or if a non-expiring token is required, create a Secret API object for the token controller to populate with a service account token by following this guide.

I used the second option which is functionally equivalent to what we had before. But maybe we should investigate how TokenRequest could be used.

Excellent. Yes, we should indeed move to a newer API at some point (I didn't check the version ranges - but those are always problematic in Kubernetes). If you wish to create tickets, that would be nice.

olim7t · 2022-09-27T19:06:21Z

Current test failures:

Run multi-cluster e2e tests / CheckStargateApisWithMultiDcCluster: looks like an unrelated timeout, and it passes locally.
Run e2e tests with isolated control plane / CreateMultiReaper: logs have expired.

I'm going to re-run them.

(from Operator SDK upgrade guide)

olim7t requested a review from a team as a code owner September 21, 2022 17:43

olim7t commented Sep 21, 2022

View reviewed changes

burmanm reviewed Sep 22, 2022

View reviewed changes

olim7t commented Sep 23, 2022

View reviewed changes

olim7t added 14 commits September 30, 2022 14:27

Update Go version everywhere

ae400c9

Port Makefile changes from cass-operator PR

f6ebd6b

Update dependencies in go.mod and run go mod tidy

81aa82e

Upgrade kube proxy image

336e838

(from Operator SDK upgrade guide)

Drop dependency to k8s.io/kubernetes

211e2fe

Update changelog

ae35509

Keep changelog entry on a single line

ea64ae9

Fix version requirements

2308544

Use BUNDLE_GEN_FLAGS

be34a55

Upgrade kuttl to 0.13.0

2150e14

Revert kind image update

f846498

Commit generated CRD changes

64423eb

Fix service account token and re-upgrade kind image

007e373

Fix changelog

5d884eb

olim7t force-pushed the upgrade-sdk branch from ec574bd to 5d884eb Compare September 30, 2022 21:28

jsanda approved these changes Sep 30, 2022

View reviewed changes

olim7t merged commit dda17aa into k8ssandra:main Sep 30, 2022

olim7t deleted the upgrade-sdk branch September 30, 2022 22:40

jsanda pushed a commit that referenced this pull request Oct 3, 2022

Upgrade to Go 1.18 and to operator-sdk 1.23 (fixes #681) (#694)

1311e80

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade to Go 1.18 and to operator-sdk 1.23 (fixes #681) #694

Upgrade to Go 1.18 and to operator-sdk 1.23 (fixes #681) #694

olim7t commented Sep 21, 2022 •

edited

olim7t left a comment

olim7t Sep 21, 2022

burmanm Sep 22, 2022

olim7t Sep 22, 2022

olim7t Sep 21, 2022

olim7t Sep 21, 2022

burmanm Sep 22, 2022

olim7t Sep 22, 2022

olim7t Sep 22, 2022 •

edited

burmanm Sep 22, 2022

codecov-commenter commented Sep 21, 2022 •

edited

burmanm Sep 22, 2022

burmanm Sep 22, 2022

olim7t Sep 22, 2022 •

edited

burmanm Sep 22, 2022

olim7t Sep 22, 2022

burmanm Sep 22, 2022

olim7t Sep 23, 2022

burmanm Sep 22, 2022

olim7t Sep 22, 2022 •

edited

burmanm Sep 22, 2022

burmanm Sep 22, 2022

burmanm left a comment

burmanm commented Sep 22, 2022

olim7t commented Sep 22, 2022

olim7t Sep 23, 2022

burmanm Sep 23, 2022

olim7t Sep 23, 2022

olim7t commented Sep 27, 2022

		github.com/bombsimon/logrusr/v2 v2.0.1
		github.com/cespare/xxhash/v2 v2.1.2 // indirect

Upgrade to Go 1.18 and to operator-sdk 1.23 (fixes #681) #694

Upgrade to Go 1.18 and to operator-sdk 1.23 (fixes #681) #694

Conversation

olim7t commented Sep 21, 2022 • edited

olim7t left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

olim7t Sep 22, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented Sep 21, 2022 • edited

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

olim7t Sep 22, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

olim7t Sep 22, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

burmanm left a comment

Choose a reason for hiding this comment

burmanm commented Sep 22, 2022

olim7t commented Sep 22, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

olim7t commented Sep 27, 2022

olim7t commented Sep 21, 2022 •

edited

olim7t Sep 22, 2022 •

edited

codecov-commenter commented Sep 21, 2022 •

edited

olim7t Sep 22, 2022 •

edited

olim7t Sep 22, 2022 •

edited