Create NS Annotation label Made a Constant #1533
Conversation
Signed-off-by: Ken Sipe <[email protected]>
| @@ -28,4 +28,6 @@ const ( | |||
|
|
|||
| // Last applied state for three way merges | |||
| LastAppliedConfigAnnotation = "kudo.dev/last-applied-configuration" | |||
|
|
|||
| CreatedByAnnotation = "kudo.dev/created-by" | |||
There was a problem hiding this comment.
The created-by semantics is somewhat strange: CLI also creates Instance, Operator, and OperatorVersion resources - should we now mark them too? Instance controller creates all other resources - should we then add kudo.dev/created-by: instance-controller?
Our labels (seen above) have been about what-this-is and not who-created-it. So maybe:
| CreatedByAnnotation = "kudo.dev/created-by" | |
| InstanceNamespaceAnnotation = "kudo.dev/instance-namespace" |
There was a problem hiding this comment.
I'm not sure we know that the namespace is for an instance... currently it is. The ns could be for an operator needs. I am simply looking for a convenient way to identify that this is a kudo managed (or created thing). This seems like the function of annotations.
I am for annotating a created-by for each of the resources you mention Instance, Operator, etc. However in these cases, there is less confusion over who created them... or more importantly who manages them. One could claim that an Instance can be created externally to kudo... with enough knowledge this would be true... but there wouldn't be any argument over who manages the Instance... it is created with the intent to have it managed by the kudo manager.
As we start to take on the creation of resources not owned by kudo. Cluster-wide resources, namespace, serviceaccount, crd, etc. It makes sense IMO to have a record as an annotation of "who" is managing or in joint management of these resources.
there could additionally be benefit in similar labels which can be used as selection... but that is a separate concern.
There was a problem hiding this comment.
However in these cases, there is less confusion over who created them... or more importantly who manages them
Fair point though I disagree about "managing" part as things are generally created by the CLI and managed by the manager. In this case, we should stay consistent and also annotate I/O/OV resources.
There was a problem hiding this comment.
I've thought about it, and I like this annotation. Should we ever introduce kudo uninstall it might become very useful. However, let's do it consistently: we should annotate all resources created by the CLI with it:
- all package resources such as
Instance,OperatorandOperatorVersion - all
kudo initresources
/cc @kensipe
There was a problem hiding this comment.
I agree... but it shouldn't need to be all in one PR right? I'll create an issue to track.
I completely agree that kudo should have an annotation on all created resources
There was a problem hiding this comment.
Is there a reason to split this functionality over multiple PRs? I imagine the whole thing being ~30 SLOCs
There was a problem hiding this comment.
Just came by this PR. Why did we introduce new annotation, just wondering? Can't we use something that we already. use for all the other objects in enhancer? Like e.g. kudo.HeritageLabel ? 🤔 sorry if you already had this discussion in the past :)
Signed-off-by: Ken Sipe [email protected]