Update golang to 1.20.5 #319
Conversation
kubevirt-bot
added
release-note-none
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Update k8s libraries to 1.27 to match golang version. Fixed unit tests to be compatible with latest fake kubernetes client. In particular updates to some core resources (such as daemonset) no longer support updating the status fields of those objects. Instead of updating in the test, delete and create with the wanted status. Signed-off-by: Alexander Wels <[email protected]>
awels
force-pushed
the
update_golang_to_120
branch
from
eb9bf04 to
e62674d
Compare
Signed-off-by: Alexander Wels <[email protected]>
| @@ -45,7 +45,8 @@ func (r *ReconcileHostPathProvisioner) reconcileSecurityContextConstraints(reqLo | |||
| } | |||
| } else { | |||
| if err := r.deleteSCC(MultiPurposeHostPathProvisionerName); err != nil { | |||
| return reconcile.Result{}, err | |||
| reqLogger.Error(err, "unable to delete SCC") | |||
There was a problem hiding this comment.
Why do we not error out anymore when the delete fails? maybe just IgnoreNotFound?
| @@ -213,7 +214,7 @@ func (r *ReconcileHostPathProvisioner) checkSCCUsed() (bool, error) { | |||
| // not using SCCs | |||
| return false, nil | |||
| } | |||
| return false, err | |||
| return false, nil | |||
There was a problem hiding this comment.
same q as above, am I missing something obvious? do we not care about SCCs anymore with CSI?
There was a problem hiding this comment.
Yeah this is the main issue I have. @awels says that is NoMatchError is no longer returned and I think that is something we should dig into and find out exactly what changed and if this is either 1) a bug 2) we should be doing something differently or 3) the new normal
There was a problem hiding this comment.
Sounds like this kubernetes-sigs/controller-runtime#2354 (comment)
BTW, did you see the disclaimer on latest controller runtime release?
https://github.com/kubernetes-sigs/controller-runtime/releases/tag/v0.15.0
| @@ -439,7 +439,7 @@ func (r *ReconcileHostPathProvisioner) checkPrometheusUsed() (bool, error) { | |||
| // prometheus not deployed | |||
| return false, nil | |||
| } | |||
| return false, err | |||
| return false, nil | |||
There was a problem hiding this comment.
I guess this is not just about SCCs
my worry with this is that we miss isErrCacheNotStarted which can cause a watch to not be set up..
I also see we ignore transient errors here in calls to checkPrometheusUsed so that
may be a problem
| @@ -90,7 +89,7 @@ func main() { | |||
| // Create a new Cmd to provide shared dependencies and start components | |||
| mgr, err := manager.New(cfg, manager.Options{ | |||
| Namespace: namespace, | |||
| MapperProvider: restmapper.NewDynamicRESTMapper, | |||
| MapperProvider: apiutil.NewDynamicRESTMapper, | |||
There was a problem hiding this comment.
People are working around issues by using this mapperprovider:
https://github.com/statnett/image-scanner-operator/pull/394/files#diff-4ddb9905d3f2a223cd9c0ccbe71976ff5ab27898b7464052462724c0c0634b2bR120
Which I think is worse than what we had before, just something to consider
|
/hold |
kubevirt-bot
added
do-not-merge/hold
|
PR needs rebase. 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. |
What this PR does / why we need it:
Update k8s libraries to 1.27 to match golang version. Fixed unit tests to be compatible with latest fake kubernetes client. In particular updates to some core resources (such as daemonset) no longer support updating the status fields of those objects. Instead of updating in the test, delete and create with the wanted status.
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 #
Special notes for your reviewer:
Release note: