| Raised in ML and waiting for the Wei Li position. ---------- Forwarded message ---------
From: Camila Macedo <cmacedo@redhat.com>
Date: Thu, May 2, 2019 at 1:48 PM
Subject: AEROGEAR-9089 - Analyse made over the request "Change the MSS operator to watch specific namespaces and/or labels instead of all as it is done in Keycloak"
To: Wei Li <weil@redhat.com>
Cc: mobile-customer-success <mobile-customer-success@redhat.com>, David Ffrench <dffrench@redhat.com>, Chris Foley <chfoley@redhat.com> Hi @folks, Following the analyse made over this topic. Keycloak It is not using the default ENV namespace and instead of it is creating and exporting a var to pass a list of NAMESPACES (Operator Namespace + MDC namespace). See here[1]
Shows that have just 1 WATCH for ALL project which will be looking for in the List of NAMESPACES(CONSUMER_NAMESPACES) and for a List of Keylock types. See here[2]
Exemplification: SELECT * FROM NAMESPACE IN [ Array ] WHERE KIND IN [ Array ]
Mobile Security Service It was solved as it's examples/docs[3] and ks8 code docs, for example[4].
In fact, the fetches/watches impl in the MSS Operator shows very more restrictive and specific which indeed should bring a better performance. See that instead of having a generic search for ALL that can be related we have ONE specific WATCH that need be checked per type and namespace.
Exemplification: SELECT "SpecificResource" FROM NAMESPACE = NAMESPACE WHERE KIND = KIND
Following a code example. // Specific watch per kind and owner type
err := c.Watch(&source.Kind{Type: &corev1.ConfigMap{}}, &handler.EnqueueRequestForOwner{
IsController: true,
OwnerType: &mobilesecurityservicev1alpha1.MobileSecurityServiceApp{},
}) {...} //Search for the ConfigMap ...
configMap := &corev1.ConfigMap{}
err := r.client.Get(context.TODO(), types.NamespacedName {Name: getSDKConfigMapName(instance), Namespace: instance.Namespace} , configMap) NOTE: The GET is very specific and is passing as parameter the namespace and the name of the resource.
All fetches implemented in the project are following this approach in order to provide a better performance and faithfully follow the same approach present in the operator-framework documentation. It is NOT searching for ALL and getting a LIST of resources to handle as shows done by keylock[9]. CONCLUSION: In POV, which leads to making this request was my silly comment in the code "//WHACH ALL NAMESPACE"[5] and really sorry for that. I had not this understanding at that moment. After a further analyse, it seems not the be the same WATCH of the resources that you point in keylock[2]. Following the doc over it. It is just for manager's cache which will be evicted by default every 10 hours. See here[7] // Namespace if specified restricts the manager's cache to watch objects in the desired namespace
// Defaults to all namespaces
// Note: If a namespace is specified then controllers can still Watch for a cluster-scoped resource e.g Node
// For namespaced resources the cache will only hold objects from the desired namespace.
Namespace string Ref.: https://github.com/kubernetes-sigs/controller-runtime/blob/master/pkg/manager/manager.go#L122 IMHO we don't need to customize it since it is just for manager's cache which will be evicted by default every 10 hours.
The watches and fetches are implemented very specifically as requested already.
The concept/nomenclature "namespace-scoped" operator show means one operator that will deal with just ONE specific namespace which appears NOT to be MSS or Keylock case since it needs to check the resources into 2 namespaces (where the operator and the app are applied). In this way, shows that following the operator-framework design it should use the ClusterRole as well. See more over it here[8]
Hi @Wei Li I understand that we could apply the "CONSUMER_NAMESPACES"[6] approach anyway if you wish in the MSS operator as well, but do you really think that it is required? Since this approach is not in the operator-framework docs then could not it be effected for any breaking change more easily? Could not this "customization" bring unnecessary complexity and make harder the understanding of the code and keep the project maintained by others? IHMO: Shows that it will not bring any significant performance result since this "customization" will be used just every10 hours and the MSS Operator is already have been very specific in these watches and fetches in order to faithfully follow the same approach present in the operator-framework documentation which should ensure a great performance. Please, let us know if you are ok with or if you still looking for we apply the "CONSUMER_NAMESPACES"[6] approach on MSS Operator as well. Really thank you for your attention and support. Cheers, [1] - https://github.com/integr8ly/keycloak-operator/search?q=CONSUMER_NAMESPACES&unscoped_q=CONSUMER_NAMESPACES [2] - https://github.com/integr8ly/keycloak-operator/blob/84904640e1d498b1d8eb9cca4b42f268728735a1/pkg/keycloak/keycloak.go#L96 [3] - https://github.com/operator-framework/getting-started#resources-watched-by-the-controller [4] - https://github.com/kubernetes-sigs/controller-runtime/blob/master/pkg/manager/manager.go#L122 [5] - https://github.com/aerogear/mobile-security-service-operator/blob/master/cmd/manager/main.go#L78 [6] - https://github.com/integr8ly/keycloak-operator/blob/b05e241da46d5de095c3108a4766d53939d041b3/deploy/operator.yaml#L31 [7] - https://github.com/kubernetes-sigs/controller-runtime/blob/master/pkg/manager/manager.go#L102 [8] - https://github.com/operator-framework/operator-sdk/blob/76d13fccc3672c0ad4eb969abb9b9885678771e4/doc/helm/user-guide.md#operator-scope [9] - https://github.com/integr8ly/keycloak-operator/blob/03300d290a424441b3ac84382f5f613c93a03934/pkg/keycloak/realm/phaseHandler.go#L67 |
