chore(deps): bump sigs.k8s.io/controller-runtime from 0.13.1 to 0.14.1 #2132
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
![dependabot[bot]](https://avatars.githubusercontent.com/in/29110?s=60&v=4){kind=small}
dependabot
bot
requested review from
mumoshu,
toast-gear and
a team
as code owners
dependabot
bot
force-pushed
the
dependabot/go_modules/sigs.k8s.io/controller-runtime-0.14.1
branch
2 times, most recently
from
ffb4b23
to
9676460
Compare
dependabot
bot
force-pushed
the
dependabot/go_modules/sigs.k8s.io/controller-runtime-0.14.1
branch
from
9676460
to
eda8be4
Compare
mumoshu
reviewed
Jan 22, 2023
| } | ||
|
|
||
| var _ = BeforeSuite(func(done Done) { |
There was a problem hiding this comment.
Since Ginkgo v2, BeforeSuite no longer accepts the function whose first parameter is Done.
This one, which takes the parameter-less function, would be the most suitable one out of all the variants explained in https://onsi.github.io/ginkgo/#suite-setup-and-cleanup-beforesuite-and-aftersuite, as we've never used the passed Done in a meaningful way.
mumoshu
reviewed
Jan 22, 2023
| @@ -681,78 +681,80 @@ func (autoscaler *HorizontalRunnerAutoscalerGitHubWebhook) SetupWithManager(mgr | |||
|
|
|||
| autoscaler.Recorder = mgr.GetEventRecorderFor(name) | |||
|
|
|||
| if err := mgr.GetFieldIndexer().IndexField(context.TODO(), &v1alpha1.HorizontalRunnerAutoscaler{}, scaleTargetKey, func(rawObj client.Object) []string { | |||
| hra := rawObj.(*v1alpha1.HorizontalRunnerAutoscaler) | |||
| if err := mgr.GetFieldIndexer().IndexField(context.TODO(), &v1alpha1.HorizontalRunnerAutoscaler{}, scaleTargetKey, autoscaler.indexer); err != nil { | |||
There was a problem hiding this comment.
The last parameter of IndexField is IndexerFunc, which is now extracted out of this function, into a new function of the *HorizontalRunnerAutoscalerGitHubWebhook so that we can use the indexer func outside of this function.
controllers/actions.summerwind.net/horizontal_runner_autoscaler_webhook_test.go is the only place we use the indexer func outside of this func, and the indexer func is used to make the test passing after the controller-runtime upgrade.
mumoshu
reviewed
Jan 22, 2023
| @@ -49,12 +48,10 @@ func TestAPIs(t *testing.T) { | |||
|
|
|||
| config.GinkgoConfig.FocusStrings = append(config.GinkgoConfig.FocusStrings, os.Getenv("GINKGO_FOCUS")) | |||
|
|
|||
| RunSpecsWithDefaultAndCustomReporters(t, | |||
| "Controller Suite", | |||
| []Reporter{printer.NewlineReporter{}}) | |||
There was a problem hiding this comment.
The whole printer package has been removed in controller-runtime v0.14.0 due to the Ginkgo v2 upgrade.
See kubernetes-sigs/controller-runtime#1977 for more context.
Also, we won't need to find an alternative to the reporter as the test output seems fine without it.
mumoshu
reviewed
Jan 22, 2023
Comment on lines
+30
to
+32
| k8s.io/api v0.26.0 | ||
| k8s.io/apimachinery v0.26.0 | ||
| k8s.io/client-go v0.26.0 |
There was a problem hiding this comment.
These Kubernetes dependency updates are introduced by the controller-runtime upgrade.
Probably, we can just close #2084 in favor of this PR.
mumoshu
reviewed
Jan 22, 2023
1 task
Bumps [sigs.k8s.io/controller-runtime](https://github.com/kubernetes-sigs/controller-runtime) from 0.13.1 to 0.14.1. - [Release notes](https://github.com/kubernetes-sigs/controller-runtime/releases) - [Changelog](https://github.com/kubernetes-sigs/controller-runtime/blob/master/RELEASE.md) - [Commits](kubernetes-sigs/controller-runtime@v0.13.1...v0.14.1) --- updated-dependencies: - dependency-name: sigs.k8s.io/controller-runtime dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
…troller-runtime controller-runtime 0.14 includes kubernetes-sigs/controller-runtime#2025 which turned out to be a breaking change for any tests that use fake controller-runtime client for list operations against the index. This commit addresses that, by extracting the indexer func out of the reconciler setup function so that both the reconciler setup func and the set-only fake client setup func can use the same indexer func. This fixes integration errors like the below example: ``` --- FAIL: TestWebhookWorkflowJobWithSelfHostedLabel (0.00s) --- FAIL: TestWebhookWorkflowJobWithSelfHostedLabel/Successful (0.00s) horizontal_runner_autoscaler_webhook_test.go:256: status: 500 horizontal_runner_autoscaler_webhook_test.go:256: body: List on GroupVersionKind actions.summerwind.dev/v1alpha1, Kind=HorizontalRunnerAutoscaler specifies selector on field scaleTarget, but no index with name scaleTarget has been registered for GroupVersionKind actions.summerwind.dev/v1alpha1, Kind=HorizontalRunnerAutoscaler horizontal_runner_autoscaler_webhook_test.go:440: diagnostics: testlog] finding repository-wide runnerrepository.name=MYREPO repository.owner.login=MYORG repository.owner.type=Organization action=queued delivery= workflowJob.labels=[self-hosted label1] workflowJob.status=queued enterprise.slug= workflowJob.runID=1234567890 workflowJob.ID=1234567890 event=workflow_job hookID= repository=MYORG/MYREPO error=List on GroupVersionKind actions.summerwind.dev/v1alpha1, Kind=HorizontalRunnerAutoscaler specifies selector on field scaleTarget, but no index with name scaleTarget has been registered for GroupVersionKind actions.summerwind.dev/v1alpha1, Kind=HorizontalRunnerAutoscaler testlog] handling check_run eventevent=workflow_job hookID= workflowJob.status=queued enterprise.slug= workflowJob.runID=1234567890 workflowJob.ID=1234567890 delivery= workflowJob.labels=[self-hosted label1] repository.name=MYREPO repository.owner.login=MYORG repository.owner.type=Organization action=queued error=List on GroupVersionKind actions.summerwind.dev/v1alpha1, Kind=HorizontalRunnerAutoscaler specifies selector on field scaleTarget, but no index with name scaleTarget has been registered for GroupVersionKind actions.summerwind.dev/v1alpha1, Kind=HorizontalRunnerAutoscaler ``` Signed-off-by: Yusuke Kuoka <ykuoka@gmail.com>
mumoshu
force-pushed
the
dependabot/go_modules/sigs.k8s.io/controller-runtime-0.14.1
branch
from
6be39f0
to
ade9ccd
Compare
TingluoHuang
approved these changes
Jan 26, 2023
|
Thanks for your review @TingluoHuang! |
mumoshu
deleted the
dependabot/go_modules/sigs.k8s.io/controller-runtime-0.14.1
branch
mumoshu
added a commit
that referenced
this pull request
Feb 26, 2023
Bumping controller-runtime from 0.14.1 to 0.14.4 (#2261) seem to break our integration test: ``` unable to install CRDs onto control plane: unable to create CRD instances: unable to create CRD "ephemeralrunners.actions.github.com": CustomResourceDefinition.apiextensions.k8s.io "ephemeralrunners.actions.github.com" is invalid: [spec.validation.openAPIV3Schema.properties[spec].properties[spec].properties[volumes].items.properties[ephemeral].properties[volumeClaimTemplate].properties[spec].properties[resources].properties[claims].x-kubernetes-list-type: Required value: must be map if x-kubernetes-list-map-keys is non-empty, spec.validation.openAPIV3Schema.properties[spec].properties[spec].properties[ephemeralContainers].items.properties[resources].properties[claims].x-kubernetes-list-type: Required value: must be map if x-kubernetes-list-map-keys is non-empty, spec.validation.openAPIV3Schema.properties[spec].properties[spec].properties[containers].items.properties[resources].properties[claims].x-kubernetes-list-type: Required value: must be map if x-kubernetes-list-map-keys is non-empty, spec.validation.openAPIV3Schema.properties[spec].properties[spec].properties[initContainers].items.properties[resources].properties[claims].x-kubernetes-list-type: Required value: must be map if x-kubernetes-list-map-keys is non-empty] ``` The offending field, "x-kubernetes-list-map-keys", was already there before the upgrade. We previously fixed a similar issue for "x-kubernetes-list-type" in #2132. At that time we already had "x-kubernetes-list-map-keys" but the integration test didn't fail. Presuming this might be due to new K8s dependencies or controller-runtime envtest change, I'd like to just drop the field like we've already done for -type field in #2132. Ref #2132 Ref #2261
mumoshu
added a commit
that referenced
this pull request
Feb 26, 2023
Bumping controller-runtime from 0.14.1 to 0.14.4 (#2261) seem to break our integration test: ``` unable to install CRDs onto control plane: unable to create CRD instances: unable to create CRD "ephemeralrunners.actions.github.com": CustomResourceDefinition.apiextensions.k8s.io "ephemeralrunners.actions.github.com" is invalid: [spec.validation.openAPIV3Schema.properties[spec].properties[spec].properties[volumes].items.properties[ephemeral].properties[volumeClaimTemplate].properties[spec].properties[resources].properties[claims].x-kubernetes-list-type: Required value: must be map if x-kubernetes-list-map-keys is non-empty, spec.validation.openAPIV3Schema.properties[spec].properties[spec].properties[ephemeralContainers].items.properties[resources].properties[claims].x-kubernetes-list-type: Required value: must be map if x-kubernetes-list-map-keys is non-empty, spec.validation.openAPIV3Schema.properties[spec].properties[spec].properties[containers].items.properties[resources].properties[claims].x-kubernetes-list-type: Required value: must be map if x-kubernetes-list-map-keys is non-empty, spec.validation.openAPIV3Schema.properties[spec].properties[spec].properties[initContainers].items.properties[resources].properties[claims].x-kubernetes-list-type: Required value: must be map if x-kubernetes-list-map-keys is non-empty] ``` To be clear, the offending field, "x-kubernetes-list-map-keys", is new, and was NOT there before the upgrade. We previously fixed a similar issue for "x-kubernetes-list-type" in #2132, by adding a post-processing step to our CRD generation make target to remove the offending fields. This commit refactors the post-processing logic into a new make target. The CRD generation target uses the new target for removing both "x-kubernetes-list-type" and the new "x-kubernetes-list-map-keys" fields. Ref #2132 Ref #2261
mumoshu
added a commit
that referenced
this pull request
Feb 26, 2023
Bumping controller-runtime from 0.14.1 to 0.14.4 (#2261) seem to break our integration test: ``` unable to install CRDs onto control plane: unable to create CRD instances: unable to create CRD "ephemeralrunners.actions.github.com": CustomResourceDefinition.apiextensions.k8s.io "ephemeralrunners.actions.github.com" is invalid: [spec.validation.openAPIV3Schema.properties[spec].properties[spec].properties[volumes].items.properties[ephemeral].properties[volumeClaimTemplate].properties[spec].properties[resources].properties[claims].x-kubernetes-list-type: Required value: must be map if x-kubernetes-list-map-keys is non-empty, spec.validation.openAPIV3Schema.properties[spec].properties[spec].properties[ephemeralContainers].items.properties[resources].properties[claims].x-kubernetes-list-type: Required value: must be map if x-kubernetes-list-map-keys is non-empty, spec.validation.openAPIV3Schema.properties[spec].properties[spec].properties[containers].items.properties[resources].properties[claims].x-kubernetes-list-type: Required value: must be map if x-kubernetes-list-map-keys is non-empty, spec.validation.openAPIV3Schema.properties[spec].properties[spec].properties[initContainers].items.properties[resources].properties[claims].x-kubernetes-list-type: Required value: must be map if x-kubernetes-list-map-keys is non-empty] ``` To be clear, the offending field, "x-kubernetes-list-map-keys", is new, and was NOT there before the upgrade. We previously fixed a similar issue for "x-kubernetes-list-type" in #2132, by adding a post-processing step to our CRD generation make target to remove the offending fields. This commit refactors the post-processing logic into a new make target. The CRD generation target uses the new target for removing both "x-kubernetes-list-type" and the new "x-kubernetes-list-map-keys" fields. Ref #2132 Ref #2261
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.
Bumps sigs.k8s.io/controller-runtime from 0.13.1 to 0.14.1.
Release notes
Sourced from sigs.k8s.io/controller-runtime's releases.
... (truncated)
Commits
84c5c9f🐛 controllers without For() fail to start (#2108)ddcb99dMerge pull request #2100 from vincepri/release-0.1469f0938Merge pull request #2094 from alvaroaleman/subresoruce-get8738e91Merge pull request #2091 from alvaroaleman/no-forca4b4deMerge pull request #2096 from lucacome/generate5673341Merge pull request #2097 from kubernetes-sigs/dependabot/go_modules/github.co...7333aed🌱 Bump github.com/onsi/ginkgo/v2 from 2.5.1 to 2.6.0d4f1e82Generate files and update modulesa387bf4Merge pull request #2093 from alvaroaleman/recover-panic-globallyda7dd5dDependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting
@dependabot rebase.Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR:
@dependabot rebasewill rebase this PR@dependabot recreatewill recreate this PR, overwriting any edits that have been made to it@dependabot mergewill merge this PR after your CI passes on it@dependabot squash and mergewill squash and merge this PR after your CI passes on it@dependabot cancel mergewill cancel a previously requested merge and block automerging@dependabot reopenwill reopen this PR if it is closed@dependabot closewill close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually@dependabot ignore this major versionwill close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this minor versionwill close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this dependencywill close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)