Bump controller-runtime, k8s libraries, openshift/library go, adjust CO to the new versions #337

jhrozek · 2023-05-30T12:12:08Z

fix(deps): update module sigs.k8s.io/controller-runtime to v0.15.0
Bump openshift/library-go
bump k8s.io/pod-security-admission to v0.27.2
bump sigs.k8s.io/controller-tools to v0.12.0
Adjust CO reconcile loop setup and unit tests for the new controller-manager version

Adjust CO reconcile loop setup and unit tests for the new controller-manager version

In order to use the latest release of controller-manager we need to use
the new style of setting up controller loops that uses
ctrl.NewControllerManagedBy() and then the builder pattern instead of
adding c.Watch for each resource.

The tests had to be adjusted, too, to specifically name resources that
can update its status.

jhrozek · 2023-05-31T11:12:32Z

/test e2e-aws-parallel

jhrozek · 2023-06-01T10:26:47Z

/test e2e-aws-parallel

rhmdnd · 2023-06-12T16:18:51Z

The parallel test failure could be related to the dependency bump:

{"level":"error","ts":"2023-06-01T11:19:05.841Z","logger":"remediationctrl","msg":"Retriable error","Request.Namespace":"osdk-e2e-80bb02e5-58c4-4085-a66c-0f85259b2217","Request.Name":"test-generic-remediation-fails-unkown","error":"failed to get API group resources: unable to retrieve the complete list of server APIs: foo.bar/v1: the server could not find the requested resource","stacktrace":"github.com/ComplianceAsCode/compliance-operator/pkg/controller/common.ReturnWithRetriableError\n\tgithub.com/ComplianceAsCode/compliance-operator/pkg/controller/common/errors.go:117\ngithub.com/ComplianceAsCode/compliance-operator/pkg/controller/complianceremediation.(*ReconcileComplianceRemediation).Reconcile\n\tgithub.com/ComplianceAsCode/compliance-operator/pkg/controller/complianceremediation/complianceremediation_controller.go:173\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Reconcile\n\tsigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:118\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler\n\tsigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:314\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem\n\tsigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:265\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2\n\tsigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:226"}
{"level":"error","ts":"2023-06-01T11:19:05.841Z","msg":"Reconciler error","controller":"complianceremediation-controller","controllerGroup":"compliance.openshift.io","controllerKind":"ComplianceRemediation","ComplianceRemediation":{"name":"test-generic-remediation-fails-unkown","namespace":"osdk-e2e-80bb02e5-58c4-4085-a66c-0f85259b2217"},"namespace":"osdk-e2e-80bb02e5-58c4-4085-a66c-0f85259b2217","name":"test-generic-remediation-fails-unkown","reconcileID":"dc56fce9-df55-490f-843c-020571cf8557","error":"failed to get API group resources: unable to retrieve the complete list of server APIs: foo.bar/v1: the server could not find the requested resource","stacktrace":"sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler\n\tsigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:324\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem\n\tsigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:265\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2\n\tsigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:226"}

The test times out after 30 minutes waiting for the remediation to error out, which it does to some extent, but doesn't register the remeidation state.

jhrozek · 2023-06-28T12:38:29Z

hmm, that test passes locally when I run that test only. But the last three CI runs of this PR fail always on that single test. Running more tests locally.

jhrozek · 2023-06-28T13:46:14Z

hmm all the parallel tests are passing locally. I'm going to re-run the tests again and watch them interactively to try and catch the error..

jhrozek · 2023-06-28T13:46:28Z

/test e2e-aws-parallel

jhrozek · 2023-06-28T14:42:46Z

I /think/ I managed to catch the issue:

{"level":"error","ts":"2023-06-28T14:38:36.312Z","logger":"remediationctrl","msg":"Retriable error","Request.Namespace":"osdk-e2e-ac518a1e-a339-4401-9a13-72e5
b2162618","Request.Name":"test-generic-remediation-fails-unkown","error":"failed to get API group resources: unable to retrieve the complete list of server APIs: foo.bar/v1: the server could not find the requested resource","stacktrace":"github.com/ComplianceAsCode/compliance-operator/pkg/controller/common.ReturnWi
thRetriableError\n\tgithub.com/ComplianceAsCode/compliance-operator/pkg/controller/common/errors.go:117\ngithub.com/ComplianceAsCode/compliance-operator/pkg/controller/complianceremediation.(*ReconcileComplianceRemediation).Reconcile\n\tgithub.com/ComplianceAsCode/compliance-operator/pkg/controller/complianceremedi
ation/complianceremediation_controller.go:173\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Reconcile\n\tsigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:118\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler\n\tsigs.k8s.io/cont
[email protected]/pkg/internal/controller/controller.go:314\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem\n\tsigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:265\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.f
unc2.2\n\tsigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:226"}                                                                  {"level":"error","ts":"2023-06-28T14:38:36.312Z","msg":"Reconciler error","controller":"complianceremediation-controller","controllerGroup":"compliance.opensh
ift.io","controllerKind":"ComplianceRemediation","ComplianceRemediation":{"name":"test-generic-remediation-fails-unkown","namespace":"osdk-e2e-ac518a1e-a339-4401-9a13-72e5b2162618"},"namespace":"osdk-e2e-ac518a1e-a339-4401-9a13-72e5b2162618","name":"test-generic-remediation-fails-unkown","reconcileID":"7c2cba67-3cd
4-4d64-9fd2-fa02cc484099","error":"failed to get API group resources: unable to retrieve the complete list of server APIs: foo.bar/v1: the server could not find the requested resource","stacktrace":"sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler\n\tsigs.k8s.io/controller-runti
[email protected]/pkg/internal/controller/controller.go:324\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem\n\tsigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:265\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2\n\tsi
gs.k8s.io/[email protected]/pkg/internal/controller/controller.go:226"}

I guess the problem is that this should not be a Retriable Error..

jhrozek · 2023-07-07T13:39:34Z

/hold
because the commit I added is a dirty hack and we should do better. But I want to see how the CI fares now (manual tests passed with the workaround)

jhrozek · 2023-07-07T13:42:17Z

pkg/controller/complianceremediation/complianceremediation_controller.go

@@ -219,7 +219,7 @@ func (r *ReconcileComplianceRemediation) reconcileRemediation(instance *compv1al
 		return common.NewNonRetriableCtrlError(
 			"Unable to get fix object from ComplianceRemediation. "+
 				"Please update the compliance-operator's permissions: %w", err)
-	} else if runtime.IsNotRegisteredError(err) || meta.IsNoMatchError(err) {
+	} else if runtime.IsNotRegisteredError(err) || meta.IsNoMatchError(err) || (err != nil && strings.Contains(err.Error(), "the server could not find the requested resource")) {


OK, so this is the ugly workaround. I cannot for the life of me figure out how to catch the proper error. In the error string chain I see unable to retrieve the complete list of server APIs which would suggest the error is ErrGroupDiscoveryFailed which should be testable for with IsGroupDiscoveryFailedError from k8s.io/client-go/discovery/discovery_client.go but that doesn't seem to work for me. Which is especially odd with go 1.20 where errors.Is should even work across wrapped errors. I'll continue trying to get to the root of the issue, but as the worst case solution, maybe this workaround would at least pass CI.

Looks like it worked for CI at least.

Strange - after looking at the implementation in client-go I'm not sure why we wouldn't be able to use:

IsGroupDiscoveryFailedError(err)

Did you happen to check the type when you were poking at this?

…manager version In order to use the latest release of controller-manager we need to use the new style of setting up controller loops that uses ctrl.NewControllerManagedBy() and then the builder pattern instead of adding c.Watch for each resource. The tests had to be adjusted, too, to specifically name resources that can update its status.

This is pretty much fluxcd/flux2@aa65589 reused. The new controller-runtime version requires that the logger is set or else there's a stack trace printed. Since even the original code doesn't use any logger for tests, let's just use null logger with the new version as well.

jhrozek · 2023-07-18T11:48:11Z

I included the patch from PR #370 as a separate patch because go mod tidy was also needed and squashing would be too messy.

rhmdnd · 2023-07-19T20:00:54Z

/test e2e-aws-parallel

rhmdnd · 2023-07-20T15:50:26Z

I'm fine merging this for now and cleaning up the error after so we can at least get updated dependencies. But, yeah, that is odd...

rhmdnd

/lgtm

openshift-ci · 2023-07-20T15:51:13Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jhrozek, rhmdnd

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~~ [jhrozek,rhmdnd]

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

rhmdnd · 2023-07-20T15:51:25Z

Adding other appropriate labels since this is a dep bump.

rhmdnd · 2023-07-20T15:52:07Z

Making sure @Vincent056 and @yuumasato have an opportunity to take a look at this before we remove the hold.

yuumasato · 2023-07-24T12:57:45Z

Making sure @Vincent056 and @yuumasato have an opportunity to take a look at this before we remove the hold.

@rhmdnd Thanks. LGTM, based on my little knowledge of Go and the operator.

rhmdnd · 2023-07-25T20:19:48Z

Getting this merged so we can cleanup the dep bump patches.

openshift-ci bot requested review from mrogers950 and rhmdnd May 30, 2023 12:12

openshift-ci bot added the approved label May 30, 2023

openshift-merge-robot added the needs-rebase label Jun 13, 2023

openshift-merge-robot removed the needs-rebase label Jun 28, 2023

openshift-ci bot added the do-not-merge/hold label Jul 7, 2023

jhrozek commented Jul 7, 2023

View reviewed changes

renovate bot and others added 9 commits July 18, 2023 13:41

fix(deps): update module sigs.k8s.io/controller-runtime to v0.15.0

774dd7a

Bump openshift/library-go

acfda34

bump k8s.io/pod-security-admission to v0.27.2

97ec6bc

bump sigs.k8s.io/controller-tools to v0.12.0

5a4d4c9

complianceremediation: Workaround for k8s changing its error types

46c71c9

Run make bundle

39a0218

Bump sigs.k8s.io/controller-tools to 0.12.1, go mod tidy

28c02dd

jhrozek mentioned this pull request Jul 18, 2023

fix(deps): update module sigs.k8s.io/controller-tools to v0.12.1 - autoclosed #370

Closed

1 task

rhmdnd approved these changes Jul 20, 2023

View reviewed changes

openshift-ci bot assigned rhmdnd Jul 20, 2023

openshift-ci bot added the lgtm label Jul 20, 2023

rhmdnd added qe-approved px-approved docs-approved labels Jul 20, 2023

rhmdnd removed the do-not-merge/hold label Jul 25, 2023

openshift-merge-robot merged commit 13ec44b into ComplianceAsCode:master Jul 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bump controller-runtime, k8s libraries, openshift/library go, adjust CO to the new versions #337

Bump controller-runtime, k8s libraries, openshift/library go, adjust CO to the new versions #337

jhrozek commented May 30, 2023

jhrozek commented May 31, 2023

jhrozek commented Jun 1, 2023

rhmdnd commented Jun 12, 2023 •

edited

Loading

jhrozek commented Jun 28, 2023

jhrozek commented Jun 28, 2023

jhrozek commented Jun 28, 2023

jhrozek commented Jun 28, 2023

jhrozek commented Jul 7, 2023

jhrozek Jul 7, 2023

rhmdnd Jul 13, 2023

rhmdnd Jul 19, 2023 •

edited

Loading

jhrozek commented Jul 18, 2023

rhmdnd commented Jul 19, 2023

rhmdnd commented Jul 20, 2023

rhmdnd left a comment

openshift-ci bot commented Jul 20, 2023

rhmdnd commented Jul 20, 2023

rhmdnd commented Jul 20, 2023

yuumasato commented Jul 24, 2023

rhmdnd commented Jul 25, 2023

Bump controller-runtime, k8s libraries, openshift/library go, adjust CO to the new versions #337

Bump controller-runtime, k8s libraries, openshift/library go, adjust CO to the new versions #337

Conversation

jhrozek commented May 30, 2023

jhrozek commented May 31, 2023

jhrozek commented Jun 1, 2023

rhmdnd commented Jun 12, 2023 • edited Loading

jhrozek commented Jun 28, 2023

jhrozek commented Jun 28, 2023

jhrozek commented Jun 28, 2023

jhrozek commented Jun 28, 2023

jhrozek commented Jul 7, 2023

jhrozek Jul 7, 2023

Choose a reason for hiding this comment

rhmdnd Jul 13, 2023

Choose a reason for hiding this comment

rhmdnd Jul 19, 2023 • edited Loading

Choose a reason for hiding this comment

jhrozek commented Jul 18, 2023

rhmdnd commented Jul 19, 2023

rhmdnd commented Jul 20, 2023

rhmdnd left a comment

Choose a reason for hiding this comment

openshift-ci bot commented Jul 20, 2023

rhmdnd commented Jul 20, 2023

rhmdnd commented Jul 20, 2023

yuumasato commented Jul 24, 2023

rhmdnd commented Jul 25, 2023

rhmdnd commented Jun 12, 2023 •

edited

Loading

rhmdnd Jul 19, 2023 •

edited

Loading