search

kudobuilder / kudo

Kubernetes Universal Declarative Operator (KUDO)
https://kudo.dev
Apache License 2.0
1.18k stars 103 forks source link

Support Instances that do not reference FrameworkVersion by namespace #375

Closed jbarrick-mesosphere closed 5 years ago

jbarrick-mesosphere commented 5 years ago

What happened:

@zen-dog and I were attempting to write a test that creates a FrameworkVersion and also uses it in the same test.

Currently, it is necessary to specify the namespace in your Instance:

kind: Instance
apiVersion: kudo.k8s.io/v1alpha1
metadata:
  name: foo-instance
  labels:
    framework: foo-framework
spec:
  frameworkVersion:
    name: foo-framework
    namespace: default

What you expected to happen:

The namespace should not be required when specifying the frameworkVersion, so that they can be deployed into any namespace without modifying the YAMLs.

jbarrick-mesosphere commented 5 years ago

@gerred do you agree with my fix, or is this something I should try to support in the test harness?

alenkacz commented 5 years ago

Is this pretty much the same as #275 ? Do we have a solution for multi-tenant cluster? I can totally see a big company having framework available for dev team but not for payroll :)

jbarrick-mesosphere commented 5 years ago

No, not the same. In this case, this just would default to checking for frameworkversions in the same namespace as the instance if a namespace is not specified. You would still be able to specify a different one if they lived in a different namespace, so this doesn't change permissions or anything.

alenkacz commented 5 years ago

oh yeah, that makes sense to me. Or is there a use case where you would want to have instance in namespace A but use FV from namespace B? 🤔 I guess if we make namespace optional it would make everyone happy which is what you're proposing so 👍

gerred commented 5 years ago

I agree with your fix @jbarrick-mesosphere we should change these to cluster CRDs anyway right?

jbarrick-mesosphere commented 5 years ago

I think so, but per #275, there's considerations around multitenancy and we aren't totally sure if we want to go that route. So I wanted to treat those as separate tasks.

gerred commented 5 years ago

Got it, ok. I agree with your fix!

gerred commented 5 years ago

Let's call this an enhancement though instead of a bug, since this is introducing something new (relaxing the requirement)