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

Upgrade problem (v0.15.2->v0.17.3): configuring namespace #2833

Closed
Svintooo opened this issue May 16, 2024 · 2 comments
Closed

Upgrade problem (v0.15.2->v0.17.3): configuring namespace #2833

Svintooo opened this issue May 16, 2024 · 2 comments

Comments

@Svintooo
Copy link

I inherited an unfamiliar code-base that broke when controller-runtime was upgraded from v0.15.2 to v0.17.3.
I think I fixed it, but I am asking for a second opinion. How I set the namespace seems unnecessary complicated.

Questions

  1. Should the below fix preserve the old functionality correctly?
  2. Is this the intended way to set the namespace and metrics bind address?

Code fix

The code that broke looked something like this:

import (
  "k8s.io/apimachinery/pkg/runtime"
  ctrl "sigs.k8s.io/controller-runtime"
)

func managerOptions(scheme *runtime.Scheme, namespace string) ctrl.Options {
  return ctrl.Options{
    Scheme:                     scheme,
    Namespace:                  namespace, // unknown field Namespace in struct literal of type manager.Options
    LeaderElection:             true,
    LeaderElectionResourceLock: "leases",
    LeaderElectionID:           "a-string-of-some-kind",
    MetricsBindAddress:         "0" // unknown field MetricsBindAddress in struct literal of type manager.Options
  }
}

My fix:

import (
  "k8s.io/apimachinery/pkg/runtime"
  ctrl "sigs.k8s.io/controller-runtime"
  "sigs.k8s.io/controller-runtime/pkg/cache" // NEW
  metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" // NEW
)

func managerOptions(scheme *runtime.Scheme, namespace string) ctrl.Options {
  return ctrl.Options{
    Scheme: scheme,
    Cache: cache.Options{ // NEW
      DefaultNamespaces: map[string]cache.Config{ // NEW
        namespace: {}, // NEW
      }, // NEW
    }, // NEW
    LeaderElection:             true,
    LeaderElectionResourceLock: "leases",
    LeaderElectionID:           "a-string-of-some-kind",
    Metrics:                    metricsserver.Options{BindAddress: "0"}, // NEW
  }
}

Extra

Nameserver fix was inspired by the following change in pkg/manager/example_test.go between v0.15.2 and v0.17.3:

opts.Namespaces = []string{"namespace1", "namespace2"}

opts.DefaultNamespaces = map[string]cache.Config{
"namespace1": {},
"namespace2": {},
}

Namespace was removed in favor of Cache: Cache.Options.Namespaces in v0.16.0 in commit e92eadb.
Cache.Options.Namespaces was changed to Cache.Options.DefaultNamespaces in v0.16.0 in commit 3e35cab.
MetricsBindAddress was changed to Metrics: metricsserver.Options.BindAddress in v0.16.0 in commit e59161e.

@sbueringer
Copy link
Member

Yup, that's correct.

@Svintooo
Copy link
Author

Thank you. Now I can be more relaxed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants