METAL-2746 check and skip if gsql instance is not ready for a new update (#74)

* METAL-2746 check and skip if gsql instance is not ready for a new update

* METAL-2746 fix unit test, cleanup makefile

* METAL-2746 replace egrep (deprecated) with grep -E

* METAL-2746 fix comments on mr

* METAL-2746 fix comments on mr

Author: dabde

Makefile

@@ -2,53 +2,47 @@
 CRD_OPTIONS ?= "crd:trivialVersions=true,preserveUnknownFields=false"
 
 .PHONY: all deploy build helm
-all: help
-
-help:
-	@echo "make miniup: start minikube"
-	@echo "make minidown: stop minikube"
-	@echo "make minidelete: delete minikube"
-	@echo "make minidashboard: open minikube dashboard"
-	@echo "make build: build db-operator docker image"
-	@echo "make helm: install helm if not exist and install local chart using helm upgrade --install command"
-	@echo "make setup: build db-operator image, install helm"
-	@echo "make update: build db-operator image again and delete running pod"
-	@echo "make addexamples: kubectl create -f examples/"
-	@echo "make test: spin up mysql, postgres containers and run go unit test"
-	@echo "make microsetup: install microk8s locally and deploy db-operator (only for linux and mac)"
-	@echo "make manifests: generate custom resource definitions"
-	@echo "make generate: generate supporting code for custom resource types"
-
-build:
+.ONESHELL: test
+
+help:   ## show this help
+	@echo 'usage: make [target] ...'
+	@echo ''
+	@echo 'targets:'
+	@grep -E '^(.+)\:\ .*##\ (.+)' ${MAKEFILE_LIST} | sort | sed 's/:.*##/#/' | column -t -c 2 -s '#'
+
+build: ## build db-operator docker image
 	@docker build -t my-db-operator:local .
 	@docker save my-db-operator > my-image.tar
 	@docker build -t mock-googleapi:local mock/
 	@docker save mock-googleapi > mock-google-api.tar
 
-helm:
+helm: ## install helm if not exist and install local chart using helm upgrade --install command
 	@helm upgrade --install --create-namespace --namespace operator my-dboperator helm/db-operator -f helm/db-operator/values.yaml -f helm/db-operator/values-local.yaml
 
-helm-lint:
+helm-lint: ## lint helm manifests
 	@helm lint -f helm/db-operator/values.yaml -f helm/db-operator/ci/ci-1.yaml --strict ./helm/db-operator
 	@helm lint -f helm/db-instances/values.yaml --strict ./helm/db-instances
 
-addexamples:
+addexamples: ## add examples via kubectl create -f examples/
 	cd ./examples/; ls | while read line; do kubectl apply -f $$line; done
 
-setup: build helm
+setup: build helm ## build db-operator image, install helm
 
 deploy:
 	@kubectl delete pod -l app=db-operator -n operator &
 	watch -n0.2 -c 'kubectl logs -l app=db-operator --all-containers=true -n operator'
 
-update: build deploy
+update: build deploy ## build db-operator image again and delete running pod
 
-test:
-	@docker-compose up -d
-	@docker-compose restart sqladmin
-	@sleep 2
-	@go test -count=1 -tags tests ./... -v -cover
-	@docker-compose down
+test: ## spin up mysql, postgres containers and run go unit test
+	tearDown() {
+		docker-compose down
+	}
+	trap tearDown EXIT
+	docker-compose up -d
+	docker-compose restart sqladmin
+	sleep 10
+	go test -count=1 -tags tests ./... -v -cover
 
 lint:
 	@golint ./...
@@ -58,23 +52,23 @@ vet:
 
 minisetup: miniup miniimage helm
 
-miniup:
+miniup: ## start minikube
 	@minikube start --kubernetes-version=v1.19.12 --cpus 2 --memory 4096
 
-minidown:
+minidown: ## stop minikube
 	@minikube stop
 
-minidelete:
+minidelete: ## delete minikube
 	@minikube delete
 
-minidashboard:
+minidashboard: ## open minikube dashboard
 	@minikube dashboard
 
 miniimage: build
 	@minikube image load my-image.tar
 	@minikube image load mock-google-api.tar
 
-k3d_setup: k3d_install k3d_image helm
+k3d_setup: k3d_install k3d_image helm ## install microk8s locally and deploy db-operator (only for linux and mac)
 
 k3d_install:
 	@wget -q -O - https://raw.githubusercontent.com/rancher/k3d/main/install.sh | bash
@@ -86,11 +80,11 @@ k3d_image: build
 	@k3d image import mock-google-api.tar -c myk3s
 
 ## Generate WebhookConfiguration, ClusterRole and CustomResourceDefinition objects.
-manifests: controller-gen
+manifests: controller-gen ## generate custom resource definitions
 	$(CONTROLLER_GEN) $(CRD_OPTIONS) rbac:roleName=manager-role webhook paths="./..." output:crd:artifacts:config=config/crd/bases
 
 ## Generate code containing DeepCopy, DeepCopyInto, and DeepCopyObject method implementations.
-generate: controller-gen
+generate: controller-gen ## generate supporting code for custom resource types
 	$(CONTROLLER_GEN) object:headerFile="hack/boilerplate.go.txt" paths="./..."
 
 CONTROLLER_GEN = $(shell pwd)/bin/controller-gen

pkg/utils/dbinstance/errors.go

@@ -23,4 +23,6 @@ var (
 	ErrAlreadyExists = errors.New("instance already exists")
 	// ErrNotExists is thrown when db instance does not exists
 	ErrNotExists = errors.New("instance does not exists")
+	// ErrInstanceNotReady is thrown when gsql instance is still not marked as Ready
+	ErrInstanceNotReady = errors.New("instance is not ready")
 )

pkg/utils/dbinstance/generic.go

@@ -66,6 +66,11 @@ func makeInterface(in *Generic) (kcidb.Database, error) {
 	}
 }
 
+func (ins *Generic) state() (string, error) {
+	logrus.Debug("generic db instance not support a state check")
+	return "NOT_SUPPORTED", nil
+}
+
 func (ins *Generic) exist() error {
 	db, err := makeInterface(ins)
 	if err != nil {

pkg/utils/dbinstance/gsql.go

@@ -202,13 +202,11 @@ func (ins *Gsql) waitUntilRunnable() error {
 	time.Sleep(delay * time.Second)
 
 	err := kci.Retry(10, 60*time.Second, func() error {
-		instance, err := ins.getInstance()
+		state, err := ins.state()
 		if err != nil {
 			return err
 		}
-		logrus.Debugf("waiting gsql instance %s state: %s", ins.Name, instance.State)
-
-		if instance.State != "RUNNABLE" {
+		if state != "RUNNABLE" {
 			return errors.New("gsql instance not ready yet")
 		}
 
@@ -226,6 +224,15 @@ func (ins *Gsql) waitUntilRunnable() error {
 	return nil
 }
 
+func (ins *Gsql) state() (string, error) {
+	instance, err := ins.getInstance()
+	if err != nil {
+		return "", err
+	}
+	logrus.Debugf("check gsql instance %s state: %s", ins.Name, instance.State)
+	return instance.State, nil
+}
+
 func (ins *Gsql) create() error {
 	err := ins.createInstance()
 	if err != nil {

pkg/utils/dbinstance/gsql_test.go

@@ -15,12 +15,11 @@ func (ins *Gsql) mockWaitUntilRunnable() error {
 
 	time.Sleep(10 * time.Second)
 
-	instance, err := ins.getInstance()
+	state, err := ins.state()
 	if err != nil {
 		return err
 	}
-
-	if instance.State != "RUNNABLE" {
+	if state != "RUNNABLE" {
 		return errors.New("gsql instance not ready yet")
 	}
 

pkg/utils/dbinstance/service.go

@@ -49,6 +49,15 @@ func Update(ins DbInstance) (map[string]string, error) {
 		return nil, ErrNotExists
 	}
 
+	state, err := ins.state()
+	if err != nil {
+		return nil, err
+	}
+
+	if state != "RUNNABLE" && state != "NOT_SUPPORTED" {
+		return nil, ErrInstanceNotReady
+	}
+
 	err = ins.update()
 	if err != nil {
 		logrus.Debug("update failed")

pkg/utils/dbinstance/types.go

@@ -21,5 +21,6 @@ type DbInstance interface {
 	exist() error
 	create() error
 	update() error
+	state() (string, error)
 	getInfoMap() (map[string]string, error)
 }