add ReadOnly and UpdateAny flags to generic controller (#23)

This commit has the following changes: - restricts generic controller to update & delete attachments to the specific watch instance which created these attachments previously - allows update to an unrelated attachment if UpdateAny is set to true - stops create, update and delete of all attachments if ReadOnly is set to true Signed-off-by: AmitKumarDas <[email protected]>
AmitKumarDas · Oct 2, 2019 · 4b8b0e7 · 4b8b0e7
1 parent b4e8a56
commit 4b8b0e7
Show file tree

Hide file tree

Showing 11 changed files with 599 additions and 294 deletions.
diff --git a/TODO b/TODO
@@ -1,42 +1,53 @@
 ### TODO:
-- make gctl integration test work
-    - code refactoring - one .go file per test
-    - proper logs, informative logs, proper log levels
+- gctl add integration tests:
+    - check if finalizer works
+    - check if attachments not created by gctl are not updated
+    - check if attachments not created by gctl are not deleted
+    - check if ReadOnly works
+    - check if UpdateAny works
+    - check if same watch can update an attachment via multiple gctl specs
+
+- gctl - read & review
+    - https://github.com/GoogleCloudPlatform/metacontroller/issues/98
+    - https://github.com/GoogleCloudPlatform/metacontroller/pull/143
+    - https://github.com/GoogleCloudPlatform/metacontroller/pull/168
+
+
+- all controller enhancements
+  - https://github.com/GoogleCloudPlatform/metacontroller/issues/154
+  - make Metac run based on either of following: 
+      - CustomResource, or # current mode
+      - Config # default mode
+  - make cctl, dctl & gctl work from config files vs. current way that depends on CRDs
+
+- explore the one binary sidecars
+    - jsonnetd service + hooks as a docker image
+
+- gctl integration test
+  - code refactoring - one .go file per test
+  - proper logs, informative logs, proper log levels  
+- Use latest stable Kubernetes version & etcd for integration testing
 - make all examples to work & then make it better
 - restructure examples to enable community
 - code to use klog
 - test/unittest
 - find bugs & fix
-- all controller enhancements
-    - https://github.com/GoogleCloudPlatform/metacontroller/issues/154
-    - make Metac run based on either of following: 
-        - CustomResource, or # current mode
-        - Config # default mode
-    - make gctl work for a list of configs i.e. configs/gctl/
-    - make cctl work for a list of configs i.e. configs/cctl/
-    - make dctl work for a list of configs i.e. configs/dctl/
-- gctl - enhancements
-    - enableCreate *bool
-    - enableDelete *bool
-    - enableApply  *bool
-    - add gctl annotation to the attachments it creates
-    - add gctl annotation to the attachments it updates
-- usecases
-    - ConformanceTest
-    - CStorConfigController
-    - DDP
-    - Install
-    - UnInstall
-    - Upgrade
-- Use latest stable Kubernetes version & etcd for integration testing
+
+- targeted usecases
+  - ConformanceTest
+  - CStorConfigController
+  - DDP
+  - Install
+  - UnInstall
+  - Upgrade
 
 ### Future Actions:
 - Make metacontroller watch resources from specific namespace if required
 - If multiple meta controllers work against a resource then apply function might be buggy
-    - Provide last-applied-configuration per meta controller resource
-    - <ctrl-ns>-<ctrl-name>.dctl.metac.openebs.io/last-applied-config
-    - <ctrl-ns>-<ctrl-name>.gctl.metac.openebs.io/last-applied-config
-    - <ctrl-ns>-<ctrl-name>.cctl.metac.openebs.io/last-applied-config
+  - Provide last-applied-configuration per meta controller resource
+  - <ctrl-ns>-<ctrl-name>.dctl.metac.openebs.io/last-applied-config
+  - <ctrl-ns>-<ctrl-name>.gctl.metac.openebs.io/last-applied-config
+  - <ctrl-ns>-<ctrl-name>.cctl.metac.openebs.io/last-applied-config
 - Should Metac support UDS?
     - https://eli.thegreenplace.net/2019/unix-domain-sockets-in-go/
 

diff --git a/apis/metacontroller/v1alpha1/types_hooks.go → apis/metacontroller/v1alpha1/hooks.go b/apis/metacontroller/v1alpha1/types_hooks.go → apis/metacontroller/v1alpha1/hooks.go
diff --git a/apis/metacontroller/v1alpha1/nameselector.go b/apis/metacontroller/v1alpha1/nameselector.go
@@ -0,0 +1,41 @@
+/*
+Copyright 2019 The MayaData Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    https://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package v1alpha1
+
+// NameSelector is used to select resources based on
+// the names set here
+type NameSelector []string
+
+// Contains returns true if the provided search item
+// is present in the selector
+func (s NameSelector) Contains(search string) bool {
+	for _, name := range s {
+		if name == search {
+			return true
+		}
+	}
+	return false
+}
+
+// ContainsOrTrue returns true if nameselector is empty or
+// search item is available in nameselector
+func (s NameSelector) ContainsOrTrue(search string) bool {
+	if len(s) == 0 {
+		return true
+	}
+	return s.Contains(search)
+}
diff --git a/apis/metacontroller/v1alpha1/types_generic.go b/apis/metacontroller/v1alpha1/types_generic.go
@@ -35,25 +35,70 @@ type GenericController struct {
 // GenericControllerSpec is the specifications for GenericController
 // API
 type GenericControllerSpec struct {
-	// Resource that is under watch by GenericController
+	// Resource that is under watch by GenericController. Any actions
+	// i.e. 'create', 'update' or 'delete' of this resource will trigger
+	// this GenericController's sync process.
 	Watch GenericControllerResource `json:"watch"`
 
-	// Attachments are the resources that get created/updated/deleted
-	// as part of formation of the desired state due to the changes
-	// in watched (i.e. above) resource
+	// Attachments are the resources that may be read, created, updated,
+	// or deleted as part of formation of the desired state. Attachments
+	// are provided along with the watch resource to the sync hooks.
+	//
+	// NOTE:
+	//	GenericController is by default limited to only update & delete
+	// the attachments that were created by its controller instance. Other
+	// attachments (i.e. the ones created via some other means) are used
+	// for readonly purposes during reconciliation.
 	Attachments []GenericControllerAttachment `json:"attachments,omitempty"`
 
-	// Hooks to be invoked to get at the desired state
+	// Hooks to be invoked to arrive at the desired state
 	Hooks *GenericControllerHooks `json:"hooks,omitempty"`
 
 	// ResyncPeriodSeconds is the time interval in seconds after which
 	// the GenericController's reconcile gets triggered. In other words
 	// this is the interval of reconciliation which runs as a continuous
 	// loop
+	//
+	// NOTE:
+	//	This is optional
 	ResyncPeriodSeconds *int32 `json:"resyncPeriodSeconds,omitempty"`
 
-	// Parameters represent a set of optional key value pairs that
-	// can take part while arriving at the desired state
+	// ReadOnly disables this controller from executing create, delete &
+	// update operations against any attachments.
+	//
+	// In other words, when set to true, GenericController can update
+	// only the watch resource & is disabled to perform any operation
+	// i.e. 'create', 'delete' or 'update' against any attachments.
+	//
+	// This can be used by sync / finalize hook implementations to read
+	// the attachments & update the watch. One should be able to perform
+	// sync operations faster in this mode, if the requirement fits this
+	// tunable.
+	//
+	// NOTE:
+	//	This is optional. However this should not be set to true if
+	// UpdateAny is set to true.
+	ReadOnly *bool `json:"readOnly,omitempty"`
+
+	// UpdateAny enables this controller to execute update operations
+	// against any attachments.
+	//
+	// NOTE:
+	//	This tunable changes the default working mode of GenericController.
+	// When set to true, the controller instance is granted with the
+	// permission to update any attachments even if these attachments
+	// were not created by this controller instance.
+	//
+	// NOTE:
+	//	This is optional. However this should not be set to true if
+	// ReadOnly is set to true.
+	UpdateAny *bool `json:"updateAny,omitempty"`
+
+	// Parameters represent a set of key value pairs that can be used by
+	// the sync hook implementation logic.
+	//
+	// NOTE:
+	//	This is optional
 	Parameters map[string]string `json:"parameters,omitempty"`
 }
 
@@ -66,30 +111,6 @@ type GenericControllerHooks struct {
 	Finalize *Hook `json:"finalize,omitempty"`
 }
 
-// NameSelector is used to select resources based on
-// the names set here
-type NameSelector []string
-
-// Contains returns true if the provided search item
-// is present in the selector
-func (s NameSelector) Contains(search string) bool {
-	for _, name := range s {
-		if name == search {
-			return true
-		}
-	}
-	return false
-}
-
-// ContainsOrTrue returns true if nameselector is empty or
-// search item is available in nameselector
-func (s NameSelector) ContainsOrTrue(search string) bool {
-	if len(s) == 0 {
-		return true
-	}
-	return s.Contains(search)
-}
-
 // GenericControllerResource represent a resource that is understood
 // by generic controller. It is used to represent a watch resource
 // as well as attachment resources.
@@ -105,21 +126,27 @@ func (s NameSelector) ContainsOrTrue(search string) bool {
 // 	A watch as well as any attachment will have its own label selector &/
 // annotation selector.
 type GenericControllerResource struct {
-	ResourceRule       `json:",inline"`
-	NameSelector       NameSelector          `json:"nameSelector,omitempty"`
-	LabelSelector      *metav1.LabelSelector `json:"labelSelector,omitempty"`
-	AnnotationSelector *AnnotationSelector   `json:"annotationSelector,omitempty"`
+	ResourceRule `json:",inline"`
+
+	// Include the resource if name selector matches
+	NameSelector NameSelector `json:"nameSelector,omitempty"`
+
+	// Include the resource if label selector matches
+	LabelSelector *metav1.LabelSelector `json:"labelSelector,omitempty"`
+
+	// Include the resource if annotation selector matches
+	AnnotationSelector *AnnotationSelector `json:"annotationSelector,omitempty"`
 }
 
 // GenericControllerAttachment represents a resources that takes
-// part in sync &/or finalize actions
+// part in sync &/or finalize.
 type GenericControllerAttachment struct {
-	// This represents a single resource that participates in the
+	// This represents the resource that should participates in
 	// sync/finalize
 	GenericControllerResource `json:",inline"`
 
-	// UpdateStrategy is the strategy to be used for the attachments
-	// after the sync/finalize process
+	// UpdateStrategy to be used for the resource to take into
+	// account the changes due to sync/finalize
 	UpdateStrategy *GenericControllerAttachmentUpdateStrategy `json:"updateStrategy,omitempty"`
 }
 

diff --git a/apis/metacontroller/v1alpha1/zz_generated.deepcopy.go b/apis/metacontroller/v1alpha1/zz_generated.deepcopy.go
diff --git a/controller/common/common.go b/controller/common/common.go
@@ -127,9 +127,9 @@ func relativeName(ref metav1.Object, obj *unstructured.Unstructured) string {
 	return obj.GetName()
 }
 
-// namespacedNameOrName returns the name of the resource based on its
+// namespaceNameOrName returns the name of the resource based on its
 // scope
-func namespacedNameOrName(obj *unstructured.Unstructured) string {
+func namespaceNameOrName(obj *unstructured.Unstructured) string {
 	if obj.GetNamespace() != "" {
 		return fmt.Sprintf(
 			"%s/%s", obj.GetNamespace(), obj.GetName(),
@@ -147,6 +147,18 @@ func describeObject(obj *unstructured.Unstructured) string {
 	return fmt.Sprintf("%s of kind %s", obj.GetName(), obj.GetKind())
 }
 
+// sanitiseAPIVersion will make the apiVersion suitable to be used
+// as value field in labels or annotations
+func sanitiseAPIVersion(version string) string {
+	return strings.ReplaceAll(version, "/", "-")
+}
+
+// DescObjAsSanitisedNSName will return the sanitised namespace name
+// format corresponding to the given object
+func DescObjAsSanitisedNSName(obj *unstructured.Unstructured) string {
+	return strings.ReplaceAll(namespaceNameOrName(obj), "/", "-")
+}
+
 // DescObjectAsKey returns a machine readable string of the provided
 // object. It can be used to identify the given object.
 func DescObjectAsKey(obj *unstructured.Unstructured) string {
@@ -162,18 +174,19 @@ func DescObjectAsKey(obj *unstructured.Unstructured) string {
 	)
 }
 
-// MakeAnnotationKeyFromObj returns a sanitised name from the
-// given object that can be used as annotation value
-func MakeAnnotationKeyFromObj(obj *unstructured.Unstructured) string {
+// DescObjectAsSanitisedKey returns a sanitised name from the
+// given object that can be used in annotation as key or value
+func DescObjectAsSanitisedKey(obj *unstructured.Unstructured) string {
+	ver := sanitiseAPIVersion(obj.GetAPIVersion())
 	ns := obj.GetNamespace()
 	if ns != "" {
 		return fmt.Sprintf("%s-%s-%s-%s",
-			obj.GetAPIVersion(), obj.GetKind(), ns, obj.GetName(),
+			ver, obj.GetKind(), ns, obj.GetName(),
 		)
 	}
 
 	return fmt.Sprintf("%s-%s-%s",
-		obj.GetAPIVersion(), obj.GetKind(), obj.GetName(),
+		ver, obj.GetKind(), obj.GetName(),
 	)
 }