Fix memory optimisation feature after upgrade to controller-runtime v0.14.5 #1543
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
k8s-ci-robot
added
release-note
k8s-ci-robot
added
approved
|
/assign @saschagrunert |
ccojocar
changed the title
|
/retest |
k8s-ci-robot
added
the
lgtm
JAORMX
approved these changes
Mar 14, 2023
|
…0.14.5 It seems that BuilderWithOptins has introduced a bug when combining the options provided by the cluster with the custom options, which breaks the memory optimization feature. BuilderWithOptions creates a new scheme object which is configured in the cache object but not in the cluster object where the original scheme object is maintained. This means that the registered schemes in the cluster scheme are not reflected into the cache. This prevents the controller from starting with error messages such as: E0314 10:44:24.541264 1302480 source.go:146] controller-runtime/source "msg"="kind must be registered to the Scheme" "error"="no kind is registered for the type v1beta1.SeccompProfile in scheme \"pkg/runtime/scheme.go:100\""
ccojocar
force-pushed
the
fix-memopt-kubernetes-upgrade
branch
from
3f58734 to
42f4bf4
Compare
k8s-ci-robot
removed
the
lgtm
|
/test all |
1 similar comment
|
/test all |
|
@saschagrunert All tests passed, please could you lgtm this again? Thanks |
saschagrunert
approved these changes
Mar 15, 2023
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ccojocar, JAORMX, saschagrunert 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:
Approvers can indicate their approval by writing |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
/kind bug
What this PR does / why we need it:
It seems that after upgrading to
controller-runtimev0.14.5, theBuilderWithOptinsfunction has introduced a bug when combiningthe options provided by the cluster with the custom options, which breaks the memory optimization feature.
BuilderWithOptionsfunction creates a new scheme object which is configured in the cache object, but not into the cluster objectwhere the original scheme object is maintained. This means that the registered schemes in the cluster scheme are not reflected
into the cache scheme. This prevents the controller from starting with this error messages:
The issue was introduced by 892ee33.
It seems that our e2e test for memory optimization didn't catch the issue
security-profiles-operator/test/tc_memoptm_test.go
Line 19 in 98d0e09
I think because we just test the start-up but in this case the spod pods got restarted after 1-2 minutes and keep restarting because of the registration issue.
Which issue(s) this PR fixes:
Does this PR have test?
Yes
Special notes for your reviewer:
Does this PR introduce a user-facing change?