Bump CAPI to v1.5.0 #4510

Ankitasw · 2023-09-21T09:41:02Z

What type of PR is this?
/kind support

What this PR does / why we need it:
Bumps CAPI to v1.5.0 based on https://github.com/kubernetes-sigs/cluster-api/blob/main/docs/book/src/developer/providers/migrations/v1.4-to-v1.5.md

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #4375
Fixes #4390

Special notes for your reviewer:

Checklist:

squashed commits
includes documentation
adds unit tests
adds or updates e2e tests

Release note:

Bump CAPI to v1.5.0

Ankitasw · 2023-09-21T09:54:46Z

/test pull-cluster-api-provider-aws-e2e
/test pull-cluster-api-provider-aws-e2e-eks

cnmcavoy · 2023-09-21T18:44:48Z

main.go

@@ -113,7 +112,6 @@ var (
 )

 func main() {
-	rand.NewSource(time.Now().UnixNano())


Why is this changing? I never noticed this line before, what was this used for?

Good point, I myself have not noticed this. I dont think this line has any affect, that's why it was removed by @furkatgofurov7

Yep. The only place we are using Random is in subnets.go to shuffle the AZs. But rand.Shuffel is already using a default seed. So it will provide pseudo random results every time its running. Removing this should be safe.

hack/ensure-go.sh

nrb · 2023-09-21T21:15:23Z

go.mod

@@ -2,7 +2,7 @@ module sigs.k8s.io/cluster-api-provider-aws/v2

 go 1.20

-replace sigs.k8s.io/cluster-api => sigs.k8s.io/cluster-api v1.4.4
+replace sigs.k8s.io/cluster-api => sigs.k8s.io/cluster-api v1.5.0


I'm confused by this - what's the effect of replacing a package with the same thing that's required?

This will pick the exact version of cluster api specified, otherwise it picks up the dirty version of whatever latest CAPI main branch has.

cc @richardcase to correct me if I am wrong.

Ankitasw · 2023-09-26T08:30:08Z

/test pull-cluster-api-provider-aws-e2e-eks

Ankitasw · 2023-09-26T11:21:13Z

All tests are passing.

AndiDog

/lgtm

with minor comments

AndiDog · 2023-09-26T14:18:14Z

.golangci.yml

-      - pkg: k8s.io/apimachinery/pkg/api/errors
-        alias: apierrors


Still used for imports, so why not lint it anymore?

This was a duplicate import in the file, so removed, otherwise we will get below error

ERRO [linters_context] invalid configuration, multiple aliases for the same package: pkg=k8s.io/apimachinery/pkg/api/errors aliases=[apierrors,apierrors] ERRO [linters_context] invalid configuration, multiple packages with the same alias: alias=apierrors packages=[k8s.io/apimachinery/pkg/api/errors,k8s.io/apimachinery/pkg/api/errors]

AndiDog · 2023-09-26T14:30:33Z

pkg/cloud/services/elb/loadbalancer_test.go

@@ -248,7 +248,7 @@ func TestGetAPIServerClassicELBSpecControlPlaneLoadBalancer(t *testing.T) {
 			expect: func(t *testing.T, g *WithT, res *infrav1.LoadBalancer) {
 				t.Helper()
 				expectedTarget := fmt.Sprintf("%v:%d", infrav1.ELBProtocolTCP, infrav1.DefaultAPIServerPort)
-				g.Expect(expectedTarget, res.HealthCheck.Target)
+				g.Expect(expectedTarget).NotTo(Equal(res.HealthCheck.Target))


Why it this now negated? Is the default target different?

Great find, thanks @AndiDog yes the default target is changed when we were moving to v1beta2 APIs. This change reside in code base for long, not sure how unit tests didn't catch it. Thanks a lot for this finding.
cc @richardcase

Skarlso · 2023-09-26T20:27:25Z

/approve
/hold
Unhold once Andi's comments are answered. :)

k8s-ci-robot · 2023-09-26T20:27:45Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Skarlso

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [Skarlso]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Ankitasw · 2023-09-27T05:39:51Z

Have added only the unit test change, so not re-running the E2E tests again.

Ankitasw · 2023-09-27T06:09:49Z

/unhold

Need one more lgtm @AndiDog @Skarlso

Skarlso · 2023-09-27T06:23:16Z

Oh dang, and that resulted in an API drift? Can we punt that?

Ankitasw · 2023-09-27T06:30:13Z

Uhmm this is weird, I just changed the unit tests, maybe some changes came with rebase thats causing the issue.

Signed-off-by: Furkat Gofurov <furkat.gofurov@suse.com>

The tests where failing after upgrading controller-runtime due to changes in the fake client. Signed-off-by: Richard Case <richard.case@outlook.com>

Signed-off-by: Furkat Gofurov <furkat.gofurov@suse.com>

Ankitasw · 2023-09-27T06:55:38Z

@Skarlso its fine now.

Skarlso · 2023-09-27T06:57:28Z

/lgtm

Ankitasw · 2023-09-27T13:58:31Z

/cherry-pick release-2.2

k8s-infra-cherrypick-robot · 2023-09-27T13:59:25Z

@Ankitasw: #4510 failed to apply on top of branch "release-2.2":

Applying: Bump CAPI to v1.5.0
Using index info to reconstruct a base tree...
M	api/v1beta2/awscluster_webhook.go
M	controllers/awsmachine_controller.go
M	controllers/awsmachine_controller_unit_test.go
M	controlplane/eks/api/v1beta2/awsmanagedcontrolplane_webhook.go
M	controlplane/eks/api/v1beta2/awsmanagedcontrolplane_webhook_test.go
M	exp/api/v1beta2/awsmanagedmachinepool_webhook.go
M	exp/api/v1beta2/awsmanagedmachinepool_webhook_test.go
M	exp/controllers/awsmachinepool_controller.go
M	go.mod
M	go.sum
M	main.go
M	pkg/internal/bytes/bytes_test.go
M	test/e2e/data/e2e_conf.yaml
M	versions.mk
Falling back to patching base and 3-way merge...
Auto-merging versions.mk
CONFLICT (content): Merge conflict in versions.mk
Auto-merging test/e2e/data/e2e_conf.yaml
Auto-merging pkg/internal/bytes/bytes_test.go
CONFLICT (content): Merge conflict in pkg/internal/bytes/bytes_test.go
Auto-merging main.go
CONFLICT (content): Merge conflict in main.go
Auto-merging go.sum
CONFLICT (content): Merge conflict in go.sum
Auto-merging go.mod
CONFLICT (content): Merge conflict in go.mod
Auto-merging exp/controllers/awsmachinepool_controller.go
Auto-merging exp/api/v1beta2/awsmanagedmachinepool_webhook_test.go
Auto-merging exp/api/v1beta2/awsmanagedmachinepool_webhook.go
Auto-merging controlplane/eks/api/v1beta2/awsmanagedcontrolplane_webhook_test.go
Auto-merging controlplane/eks/api/v1beta2/awsmanagedcontrolplane_webhook.go
Auto-merging controllers/awsmachine_controller_unit_test.go
Auto-merging controllers/awsmachine_controller.go
Auto-merging api/v1beta2/awscluster_webhook.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Bump CAPI to v1.5.0
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherry-pick release-2.2

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Ankitasw requested a review from richardcase September 21, 2023 09:41

k8s-ci-robot requested review from shivi28 and Skarlso September 21, 2023 09:41

Ankitasw requested review from wyike and removed request for Skarlso and shivi28 September 21, 2023 09:41

Ankitasw mentioned this pull request Sep 21, 2023

Bump CAPI to v1.5.0 #4398

Closed

4 tasks

Ankitasw changed the title ~~Bump capi v1.5~~ Bump CAPI v1.5 Sep 21, 2023

Ankitasw changed the title ~~Bump CAPI v1.5~~ Bump CAPI to v1.5.0 Sep 21, 2023

Ankitasw force-pushed the bump-capi-v1.5 branch 8 times, most recently from 485a62d to 6a92789 Compare September 21, 2023 15:57

cnmcavoy reviewed Sep 21, 2023

View reviewed changes

nrb reviewed Sep 21, 2023

View reviewed changes

Ankitasw force-pushed the bump-capi-v1.5 branch 5 times, most recently from 416e708 to 1913959 Compare September 22, 2023 10:08

AndiDog reviewed Sep 26, 2023

View reviewed changes

k8s-ci-robot assigned AndiDog Sep 26, 2023

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 26, 2023

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 26, 2023

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 26, 2023

Ankitasw force-pushed the bump-capi-v1.5 branch from 859c7ad to 0173b10 Compare September 27, 2023 05:39

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 27, 2023

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 27, 2023

furkatgofurov7 and others added 5 commits September 27, 2023 12:03

Bump CAPI to v1.5.0

8167d6d

Signed-off-by: Furkat Gofurov <furkat.gofurov@suse.com>

Bump golang to 1.20.6

1aaf01d

Signed-off-by: Furkat Gofurov <furkat.gofurov@suse.com>

test: fix tests after bumping controller-runtime

602ba5c

The tests where failing after upgrading controller-runtime due to changes in the fake client. Signed-off-by: Richard Case <richard.case@outlook.com>

Bump golangci-lint to v1.53.3 and make necessary changes

9d72308

Signed-off-by: Furkat Gofurov <furkat.gofurov@suse.com>

Bump controller tools dependancy to v0.12.1 and generate CRDs

43b06c8

Signed-off-by: Furkat Gofurov <furkat.gofurov@suse.com>

Ankitasw force-pushed the bump-capi-v1.5 branch from 0173b10 to 43b06c8 Compare September 27, 2023 06:33

k8s-ci-robot assigned Skarlso Sep 27, 2023

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 27, 2023

k8s-ci-robot merged commit 135a3ff into kubernetes-sigs:main Sep 27, 2023
13 checks passed

damdo mentioned this pull request Oct 5, 2023

Broken tests for unused import for cloud/services/{network,ec2} in v2.2.4 #4546

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bump CAPI to v1.5.0 #4510

Bump CAPI to v1.5.0 #4510

Ankitasw commented Sep 21, 2023 •

edited

Ankitasw commented Sep 21, 2023

cnmcavoy Sep 21, 2023

Ankitasw Sep 22, 2023

Skarlso Sep 26, 2023

nrb Sep 21, 2023

Ankitasw Sep 22, 2023

Ankitasw Sep 22, 2023

Ankitasw commented Sep 26, 2023

Ankitasw commented Sep 26, 2023

AndiDog left a comment

AndiDog Sep 26, 2023

Ankitasw Sep 27, 2023

AndiDog Sep 26, 2023

Ankitasw Sep 27, 2023 •

edited

Skarlso commented Sep 26, 2023

k8s-ci-robot commented Sep 26, 2023

Ankitasw commented Sep 27, 2023

Ankitasw commented Sep 27, 2023

Skarlso commented Sep 27, 2023

Ankitasw commented Sep 27, 2023

Ankitasw commented Sep 27, 2023

Skarlso commented Sep 27, 2023

Ankitasw commented Sep 27, 2023

k8s-infra-cherrypick-robot commented Sep 27, 2023

Bump CAPI to v1.5.0 #4510

Bump CAPI to v1.5.0 #4510

Conversation

Ankitasw commented Sep 21, 2023 • edited

Ankitasw commented Sep 21, 2023

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

Ankitasw commented Sep 26, 2023

Ankitasw commented Sep 26, 2023

AndiDog 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

Ankitasw Sep 27, 2023 • edited

Choose a reason for hiding this comment

Skarlso commented Sep 26, 2023

k8s-ci-robot commented Sep 26, 2023

Ankitasw commented Sep 27, 2023

Ankitasw commented Sep 27, 2023

Skarlso commented Sep 27, 2023

Ankitasw commented Sep 27, 2023

Ankitasw commented Sep 27, 2023

Skarlso commented Sep 27, 2023

Ankitasw commented Sep 27, 2023

k8s-infra-cherrypick-robot commented Sep 27, 2023

Ankitasw commented Sep 21, 2023 •

edited

Ankitasw Sep 27, 2023 •

edited