noobaa-operator icon indicating copy to clipboard operation
noobaa-operator copied to clipboard

Multi Namespace BucketClass

Open tangledbytes opened this issue 2 years ago • 6 comments

Explain the changes

This PR intends to add support for creating BucketClass in any namespace. The changes are made only in the controllers, if approved then appropriate changes will be propagated to NooBaa CLI in another PR.

Brief Overview With this PR:

  • noobaa operator has 2 managers now:
    • First manager is namespace scoped and monitors the namespace scoped objects (rest of NooBaa resources).
    • Second manager is cluster scoped and monitors entire cluster (for now BucketClass only).
  • BucketClass can be created in any namespace.
    • BucketClass can choose noobaa system by using "noobaa-operator" label in the metadata.
    • If no label is specified then NooBaa system is searched in the namespace of the BucketClass. This ensures backwards compatibility.
    • BucketClass with invalid provisioners are ignored.
  • OBC can refer to BucketClass in either their namespace or in NooBaa system namespace
    • If there is a conflict, BucketClass in OBC namespace is chosen over BucketClass in NooBaa system namespace.

Testing Instructions:

  1. make test-cli-flow
  • [x] Doc added/updated
  • [x] Tests added

tangledbytes avatar Mar 28 '22 05:03 tangledbytes

I think I can squash the commits after addressing the comments on this PR.

tangledbytes avatar Mar 29 '22 11:03 tangledbytes

@romayalon,

@utkarsh-pro WOW! what an awesome PR, very clean and sharp :) I have some concerns I want to put here for discussion:

  1. Admission controller - 1.1. We would probably want to add the new validations, @kfir-payne right? 1.2. I saw usage in the admission server/validator code of util.GetWatchNamespace() - we should check that nothing breaks.

I think the only time util.GetWatchNamespace() can cause issue if we are relying on getting an error when WATCH_NAMESPACE="" 🤔 .

  1. None CLI installations - (for example in OCS operator) - how will it work?
    This features requires 2 things to be in place to work properly:
  1. WATCH_NAMESPACE="" - We can modify this in the OCS operator (hard coding). The implications being that every OCS operator deployed noobaa will have this feature on by default (not sure if this is desirable).
  2. Escalation of certain privileges - We can change the noobaa operator CSV (noobaa olm csv is bundled with OCS operator as far as I could understand). Again this means that noobaa deployments will have escalated privileges (again, not sure if desirable or not).

The above suggestion reflect my lack of understanding of flexibility that OLM along with OCS may provide. My understanding is that we can't change bundled CSVs (for privileges) and Subscriptions (for env).

  1. Uninstallation flows - will Bucketclass still be deleted if it's on a different namespace?

At the moment, no. Should I delete all of the BucketClass which have the appropriate provisioner label?

  1. AllowedNamespace restriction - it is the only way for restricting bucketclass of using backingstores/namespacestores, should we add another filter in the namespace for restriction? maybe prefix?

Just to clarify, the proposal is to also have something like allowedNamespacePrefixes: noobaa-devs,noobaa-qe so that bucketclass in namespace that starts with noobaa-devs- and noobaa-qe- can refer appropriate backingstore and namespacestore?

tangledbytes avatar May 28 '22 11:05 tangledbytes

@utkarsh-pro So according to your last comment, what should/shouldn't be taken care of? In my opinion the 2 most critical things are the Non cli installation and uninstallation

romayalon avatar Jun 02 '22 07:06 romayalon

The CI tests are failing with error Exiting due to GUEST_MISSING_CONNTRACK: Sorry, Kubernetes 1.20.2 requires conntrack to be installed in root's path. Do we need to update our workflow?

tangledbytes avatar Jun 10 '22 06:06 tangledbytes

NooBaa Team Discussion Summary 8th Sept

  1. Minimise any type of access control (namely, allowedNamespaces annotations) in this iteration (i.e iteration 1).
    • Rationale: May lead to regressions and unwanted hassles in the future.
  2. Restrict the OBC reference of BucketClass to its own namespace or to NooBaa operator namespace.
    • Rationale: In future iterations, we would want to expand "multi namespace" concept to other NooBaa resources as well, in such case we anyhow will be prohibiting cross namespace reference.
  3. Multiple Manager approach is to be tested and ensured that we don't encounter any issues in the ODF Metrics.
    • Rationale: 2 manager approach is being tested, one for cluster scoped resources and another for namespace scoped resources. This is to ensure that we escalate privilege only for the the resources we want to observe cluster wide.

tangledbytes avatar Sep 08 '22 08:09 tangledbytes

@dannyzaken Can you please review it?

liranmauda avatar Sep 20 '22 08:09 liranmauda

@dannyzaken can you please review?

liranmauda avatar Nov 22 '22 09:11 liranmauda