Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(deps): bump sigs.k8s.io/controller-runtime from 0.13.1 to 0.14.1 #2132

Merged

Conversation

dependabot[bot]
Copy link
Contributor

@dependabot dependabot bot commented on behalf of github Jan 9, 2023

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.

v0.14.1

Changes since v0.14.0

πŸ› Bug Fixes

Full Changelog: kubernetes-sigs/controller-runtime@v0.14.0...v0.14.1

v0.14.0

Changes since v0.13.1

⚠️ Breaking Changes

  • Add Get functionality to SubResourceClient (#2094)
  • Allow configuring RecoverPanic for controllers globally (#2093)
  • Add client.SubResourceWriter (#2072)
  • Support registration and removal of event handler (#2046)
  • Update Kubernetes dependencies to v0.26 (#2043, #2087)
  • Zap log: Default to RFC3339 time encoding (#2029)
  • cache.BuilderWithOptions inherit options from caller (#1980)
  • Migrate to ginkgo v2 (kubernetes-sigs/controller-runtime#1977), this removes the pkg/envtest/printer package

✨ New Features

  • Builder: Do not require For (#2091)
  • support disable deepcopy on list funcion (#2076)
  • Add cluster.NewClientFunc with options (#2054)
  • Tidy up startup logging of kindWithCache source (#2057)
  • Add function to get reconcileID from context (#2056)
  • feat: add NOT predicate (#2031)
  • Allow to provide a custom lock interface to manager (#2027)
  • Add tls options to manager.Options (#2023)
  • Update Go version to 1.19 (#1986)

πŸ› Bug Fixes

  • Prevent manager from getting started a second time (#2090)
  • Missing error log for in-cluster config (#2051)
  • Skip custom mutation handler when delete a CR (#2049)
  • fix: improve semantics of combining cache selectorsByObject (#2039)
  • Conversion webhook should not panic when conversion request is nil (#1970)

🌱 Others

  • Prepare for release 0.14 (#2100)
  • Generate files and update modules (#2096)
  • Bump github.com/onsi/ginkgo/v2 from 2.5.1 to 2.6.0 (#2097)
  • Bump golang.org/x/time (#2089)
  • Update OWNERS: remove inactive members, promote fillzpp sbueringer (#2088, #2092)
  • Default ENVTEST version to a working one (1.24.2) (#2081)
  • Update golangci-lint to v1.50.1 (#2080)
  • Bump go.uber.org/zap from 1.23.0 to 1.24.0 (#2077)
  • Bump golang.org/x/sys from 0.2.0 to 0.3.0 (#2078)

... (truncated)

Commits
  • 84c5c9f πŸ› controllers without For() fail to start (#2108)
  • ddcb99d Merge pull request #2100 from vincepri/release-0.14
  • 69f0938 Merge pull request #2094 from alvaroaleman/subresoruce-get
  • 8738e91 Merge pull request #2091 from alvaroaleman/no-for
  • ca4b4de Merge pull request #2096 from lucacome/generate
  • 5673341 Merge 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.0
  • d4f1e82 Generate files and update modules
  • a387bf4 Merge pull request #2093 from alvaroaleman/recover-panic-globally
  • da7dd5d ⚠️ Allow configuring RecoverPanic for controllers globally
  • Additional commits viewable in compare view

Dependabot compatibility score

Dependabot 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 rebase will rebase this PR
  • @dependabot recreate will recreate this PR, overwriting any edits that have been made to it
  • @dependabot merge will merge this PR after your CI passes on it
  • @dependabot squash and merge will squash and merge this PR after your CI passes on it
  • @dependabot cancel merge will cancel a previously requested merge and block automerging
  • @dependabot reopen will reopen this PR if it is closed
  • @dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
  • @dependabot ignore this major version will 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 version will 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 dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)

@dependabot dependabot bot requested review from mumoshu, toast-gear and a team as code owners January 9, 2023 12:52
@dependabot 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 January 17, 2023 17:11
@dependabot dependabot bot force-pushed the dependabot/go_modules/sigs.k8s.io/controller-runtime-0.14.1 branch from 9676460 to eda8be4 Compare January 21, 2023 03:57
@mumoshu mumoshu self-assigned this Jan 22, 2023
}

var _ = BeforeSuite(func(done Done) {
Copy link
Collaborator

@mumoshu mumoshu Jan 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@@ -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 {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@@ -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{}})
Copy link
Collaborator

@mumoshu mumoshu Jan 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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
Copy link
Collaborator

@mumoshu mumoshu Jan 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These Kubernetes dependency updates are introduced by the controller-runtime upgrade.
Probably, we can just close #2084 in favor of this PR.

Makefile Show resolved Hide resolved
dependabot bot and others added 5 commits January 23, 2023 22:46
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 mumoshu force-pushed the dependabot/go_modules/sigs.k8s.io/controller-runtime-0.14.1 branch from 6be39f0 to ade9ccd Compare January 23, 2023 22:47
@mumoshu
Copy link
Collaborator

mumoshu commented Jan 27, 2023

Thanks for your review @TingluoHuang!

@mumoshu mumoshu merged commit 219ba5b into master Jan 27, 2023
@mumoshu mumoshu deleted the dependabot/go_modules/sigs.k8s.io/controller-runtime-0.14.1 branch January 27, 2023 00:23
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants