Improve recreation of instances (#48)

* add linting support

* fix linting issue

* add documentation for annotations

* Implement exponential back-off for recreation

* add generated files

* use suite_test to support several integrations tests

* Update controll tool version and update CRDs

* Add the tests for Service instance controller

* Update internal/controllers/suite_test.go

Co-authored-by: uwefreidank <49634966+uwefreidank@users.noreply.github.com>

* Adopted the suggession  for a test

* Add generated file api/v1alpha1/zz_generated.deepcopy.go

* reverting back the changes to original code for create instance fails test

* Make infinite retries as the default behavior

* Added test for infinite retry

* Adding comment for use of the reconcileTimeout

* Update documentation of the annotations

---------

Co-authored-by: RalfHammer <119853077+RalfHammer@users.noreply.github.com>
Co-authored-by: shilparamasamyreddy <164521358+shilparamasamyreddy@users.noreply.github.com>
Co-authored-by: uwefreidank <49634966+uwefreidank@users.noreply.github.com>

.vscode/launch.json

@@ -27,6 +27,7 @@
           "console": "internalConsole",
           "env": {
             "KUBEBUILDER_ASSETS": "${workspaceFolder}/bin/k8s/current",
+            "TEST_TIMEOUT": "1200",
           },
           "args": [],
           "showLog": true

Makefile

@@ -6,9 +6,9 @@ ENVTEST_K8S_VERSION = 1.25.0
 
 # Get the currently used golang install path (in GOPATH/bin, unless GOBIN is set)
 ifeq (,$(shell go env GOBIN))
-GOBIN=$(shell go env GOPATH)/bin
+  GOBIN=$(shell go env GOPATH)/bin
 else
-GOBIN=$(shell go env GOBIN)
+  GOBIN=$(shell go env GOBIN)
 endif
 
 # Setting SHELL to bash allows bash commands to be executed by recipes.
@@ -57,11 +57,16 @@ fmt: ## Run go fmt against code.
 vet: ## Run go vet against code.
 	go vet ./...
 
+# Run the linter against the code
+.PHONY: lint
+lint: golangci-lint ## Run linter against the code
+	$(GOLINT) run ./...
+
 .PHONY: test
 test: manifests generate fmt vet envtest ## Run tests.
 	KUBEBUILDER_ASSETS="$(LOCALBIN)/k8s/current" go test ./... -coverprofile cover.out
 
-.PHONY: test-fast ## Run tests without build.
+.PHONY: manifests test-fast ## Run tests without build.
 test-fast: envtest ## Run tests.
 	KUBEBUILDER_ASSETS="$(LOCALBIN)/k8s/current" go test ./... -coverprofile cover.out -ginkgo.v
 
@@ -133,20 +138,26 @@ LOCALBIN ?= $(shell pwd)/bin
 $(LOCALBIN):
 	mkdir -p $(LOCALBIN)
 
+.PHONY: clean
+clean:
+	rm -rf $(LOCALBIN)
+
 ## Tool Binaries
 KUSTOMIZE ?= $(LOCALBIN)/kustomize
 CONTROLLER_GEN ?= $(LOCALBIN)/controller-gen
 COUNTERFEITER_GEN ?= $(LOCALBIN)/counterfeiter-gen
+GOLINT ?= $(LOCALBIN)/golangci-lint
 ENVTEST ?= $(LOCALBIN)/setup-envtest
 CLIENT_GEN ?= $(shell pwd)/bin/client-gen
 INFORMER_GEN ?= $(shell pwd)/bin/informer-gen
 LISTER_GEN ?= $(shell pwd)/bin/lister-gen
 
 ## Tool Versions
 KUSTOMIZE_VERSION ?= v3.8.7
-CONTROLLER_TOOLS_VERSION ?= v0.9.2
+CONTROLLER_TOOLS_VERSION ?= v0.14.0
 CODE_GENERATOR_VERSION ?= v0.23.4
 COUNTERFEITER_VERSION ?= v6.8.1
+GOLINT_VERSION ?= v1.57.1
 
 KUSTOMIZE_INSTALL_SCRIPT ?= "https://raw.githubusercontent.com/kubernetes-sigs/kustomize/master/hack/install_kustomize.sh"
 .PHONY: kustomize
@@ -164,6 +175,12 @@ counterfeiter-gen: $(COUNTERFEITER_GEN) ## Download controller-gen locally if ne
 $(COUNTERFEITER_GEN): $(LOCALBIN)
 	test -s $(COUNTERFEITER_GEN) || GOBIN=$(LOCALBIN) go install github.com/maxbrunsfeld/counterfeiter/v6@$(COUNTERFEITER_VERSION)
 
+# find or download golangci-lint
+.PHONY: golangci-lint
+golangci-lint: $(GOLINT)
+$(GOLINT): $(LOCALBIN)
+	test -s $(GOLINT) || GOBIN=$(LOCALBIN) go install github.com/golangci/golangci-lint/cmd/golangci-lint@$(GOLINT_VERSION)
+
 .PHONY: envtest
 envtest: $(ENVTEST) ## Download envtest-setup locally if necessary.
 $(ENVTEST): $(LOCALBIN)

api/v1alpha1/constants.go

@@ -10,4 +10,11 @@ const (
 	LabelKeyClusterSpace    = "service-operator.cf.cs.sap.com/cluster-space"
 	LabelKeyServiceInstance = "service-operator.cf.cs.sap.com/service-instance"
 	LabelKeyServiceBinding  = "service-operator.cf.cs.sap.com/service-binding"
+
+	// annotation on custom resources
+	AnnotationRecreate = "service-operator.cf.cs.sap.com/recreate-on-creation-failure"
+	// annotation max number of retries for a failed operation on a service instance
+	AnnotationMaxRetries = "service-operator.cf.cs.sap.com/max-retries"
+	// annotation to hold the reconciliation timeout value
+	AnnotationReconcileTimeout = "service-operator.cf.cs.sap.com/timeout-on-reconcile"
 )

api/v1alpha1/serviceinstance_types.go

@@ -120,6 +120,16 @@ type ServiceInstanceStatus struct {
 	// +optional
 	ServiceInstanceDigest string `json:"serviceInstanceDigest,omitempty"`
 
+	// Counts the number of retries that have been attempted for the reconciliation of this service instance.
+	// This counter can be used to fail the instance if too many retries occur.
+	// +optional
+	RetryCounter int `json:"retryCounter,omitempty"`
+
+	// This is the maximum number of retries that are allowed for the reconciliation of this service instance.
+	// If the retry counter exceeds this value, the service instance will be marked as failed.
+	// +optional
+	MaxRetries int `json:"maxRetries,omitempty"`
+
 	// List of status conditions to indicate the status of a ServiceInstance.
 	// Known condition types are `Ready`.
 	// +optional

config/crd/bases/cf.cs.sap.com_clusterspaces.yaml

@@ -3,8 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.9.2
-  creationTimestamp: null
+    controller-gen.kubebuilder.io/version: v0.14.0
   name: clusterspaces.cf.cs.sap.com
 spec:
   group: cf.cs.sap.com
@@ -28,14 +27,19 @@ spec:
         description: ClusterSpace is the Schema for the clusterspaces API
         properties:
           apiVersion:
-            description: 'APIVersion defines the versioned schema of this representation
-              of an object. Servers should convert recognized schemas to the latest
-              internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources'
+            description: |-
+              APIVersion defines the versioned schema of this representation of an object.
+              Servers should convert recognized schemas to the latest internal value, and
+              may reject unrecognized values.
+              More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources
             type: string
           kind:
-            description: 'Kind is a string value representing the REST resource this
-              object represents. Servers may infer this from the endpoint the client
-              submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
+            description: |-
+              Kind is a string value representing the REST resource this object represents.
+              Servers may infer this from the endpoint the client submits requests to.
+              Cannot be updated.
+              In CamelCase.
+              More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds
             type: string
           metadata:
             type: object
@@ -48,18 +52,21 @@ spec:
                 minLength: 1
                 type: string
               guid:
-                description: Space GUID. Must not be specified if Name or OrganizationName
-                  is present.
+                description: |-
+                  Space GUID.
+                  Must not be specified if Name or OrganizationName is present.
                 minLength: 1
                 type: string
               name:
-                description: Space name. Must not be specified if Guid is present;
-                  defauls to metadata.name otherwise.
+                description: |-
+                  Space name.
+                  Must not be specified if Guid is present; defauls to metadata.name otherwise.
                 minLength: 1
                 type: string
               organizationName:
-                description: Organization name. Must not be specified if Guid is present;
-                  required otherwise.
+                description: |-
+                  Organization name.
+                  Must not be specified if Guid is present; required otherwise.
                 minLength: 1
                 type: string
             required:
@@ -71,24 +78,28 @@ spec:
             description: SpaceStatus defines the observed state of Space.
             properties:
               conditions:
-                description: List of status conditions to indicate the status of a
-                  Space. Known condition types are `Ready`.
+                description: |-
+                  List of status conditions to indicate the status of a Space.
+                  Known condition types are `Ready`.
                 items:
                   description: SpaceCondition contains condition information for a
                     Space.
                   properties:
                     lastTransitionTime:
-                      description: LastTransitionTime is the timestamp corresponding
-                        to the last status change of this condition.
+                      description: |-
+                        LastTransitionTime is the timestamp corresponding to the last status
+                        change of this condition.
                       format: date-time
                       type: string
                     message:
-                      description: Message is a human readable description of the
-                        details of the last transition, complementing reason.
+                      description: |-
+                        Message is a human readable description of the details of the last
+                        transition, complementing reason.
                       type: string
                     reason:
-                      description: Reason is a brief machine readable explanation
-                        for the condition's last transition.
+                      description: |-
+                        Reason is a brief machine readable explanation for the condition's last
+                        transition.
                       type: string
                     status:
                       description: Status of the condition, one of ('True', 'False',

config/crd/bases/cf.cs.sap.com_servicebindings.yaml

@@ -3,8 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.9.2
-  creationTimestamp: null
+    controller-gen.kubebuilder.io/version: v0.14.0
   name: servicebindings.cf.cs.sap.com
 spec:
   group: cf.cs.sap.com
@@ -28,14 +27,19 @@ spec:
         description: ServiceBinding is the Schema for the servicebindings API
         properties:
           apiVersion:
-            description: 'APIVersion defines the versioned schema of this representation
-              of an object. Servers should convert recognized schemas to the latest
-              internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources'
+            description: |-
+              APIVersion defines the versioned schema of this representation of an object.
+              Servers should convert recognized schemas to the latest internal value, and
+              may reject unrecognized values.
+              More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources
             type: string
           kind:
-            description: 'Kind is a string value representing the REST resource this
-              object represents. Servers may infer this from the endpoint the client
-              submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
+            description: |-
+              Kind is a string value representing the REST resource this object represents.
+              Servers may infer this from the endpoint the client submits requests to.
+              Cannot be updated.
+              In CamelCase.
+              More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds
             type: string
           metadata:
             type: object
@@ -48,13 +52,14 @@ spec:
                 minLength: 1
                 type: string
               parameters:
-                description: Binding parameters. Do not provide any sensitve data
-                  here; instead use ParametersFrom for such data.
+                description: |-
+                  Binding parameters.
+                  Do not provide any sensitve data here; instead use ParametersFrom for such data.
                 x-kubernetes-preserve-unknown-fields: true
               parametersFrom:
-                description: References to secrets containing binding parameters.
-                  Top level keys must occur only once across Parameters and the secrest
-                  listed here.
+                description: |-
+                  References to secrets containing binding parameters.
+                  Top level keys must occur only once across Parameters and the secrest listed here.
                 items:
                   description: ParametersFromSource represents the source of a set
                     of Parameters
@@ -77,21 +82,21 @@ spec:
                   type: object
                 type: array
               secretKey:
-                description: Secret key (referring to SecretName) where the binding
-                  credentials will be stored. If unspecified, the top level keys of
-                  the binding credentials will become the secret keys.
+                description: |-
+                  Secret key (referring to SecretName) where the binding credentials will be stored.
+                  If unspecified, the top level keys of the binding credentials will become the secret keys.
                 minLength: 1
                 type: string
               secretName:
-                description: Secret name where the binding credentials shall be stored
-                  (in the same namespace where the binding exists). If unspecified,
-                  metadata.name will be used.
+                description: |-
+                  Secret name where the binding credentials shall be stored (in the same namespace where the binding exists).
+                  If unspecified, metadata.name will be used.
                 minLength: 1
                 type: string
               serviceInstanceName:
-                description: Name of a ServiceInstance resource in the same namespace,
-                  identifying the Cloud Foundry service instance this binding refers
-                  to.
+                description: |-
+                  Name of a ServiceInstance resource in the same namespace,
+                  identifying the Cloud Foundry service instance this binding refers to.
                 minLength: 1
                 type: string
             required:
@@ -103,24 +108,28 @@ spec:
             description: ServiceBindingStatus defines the observed state of ServiceBinding
             properties:
               conditions:
-                description: List of status conditions to indicate the status of a
-                  ServiceBinding. Known condition types are `Ready`.
+                description: |-
+                  List of status conditions to indicate the status of a ServiceBinding.
+                  Known condition types are `Ready`.
                 items:
                   description: ServiceBindingCondition contains condition information
                     for a ServiceBinding.
                   properties:
                     lastTransitionTime:
-                      description: LastTransitionTime is the timestamp corresponding
-                        to the last status change of this condition.
+                      description: |-
+                        LastTransitionTime is the timestamp corresponding to the last status
+                        change of this condition.
                       format: date-time
                       type: string
                     message:
-                      description: Message is a human readable description of the
-                        details of the last transition, complementing reason.
+                      description: |-
+                        Message is a human readable description of the details of the last
+                        transition, complementing reason.
                       type: string
                     reason:
-                      description: Reason is a brief machine readable explanation
-                        for the condition's last transition.
+                      description: |-
+                        Reason is a brief machine readable explanation for the condition's last
+                        transition.
                       type: string
                     status:
                       description: Status of the condition, one of ('True', 'False',