Fix communication with ArangoD API in version 3.5.1 (#465)

ajanikow · web-flow · commit 89ab9a234441 · 2019-10-16T16:58:20.000+02:00
diff --git a/chart/kube-arangodb/templates/deployment.yaml b/chart/kube-arangodb/templates/deployment.yaml
@@ -49,6 +49,9 @@ spec:
             hostNetwork: false
             hostPID: false
             hostIPC: false
+            securityContext:
+                runAsNonRoot: true
+                runAsUser: 1000
             containers:
                 - name: operator
                   imagePullPolicy: {{ .Values.operator.imagePullPolicy }}
diff --git a/chart/kube-arangodb/templates/storage-operator/cluster-role.yaml b/chart/kube-arangodb/templates/storage-operator/cluster-role.yaml
@@ -13,6 +13,9 @@ metadata:
         app.kubernetes.io/instance: {{ .Release.Name }}
         release: {{ .Release.Name }}
 rules:
+    - apiGroups: [""]
+      resources: ["persistentvolumes", "persistentvolumeclaims", "endpoints", "events", "services"]
+      verbs: ["*"]
     - apiGroups: ["apiextensions.k8s.io"]
       resources: ["customresourcedefinitions"]
       verbs: ["get"]
diff --git a/chart/kube-arangodb/templates/storage-operator/role.yaml b/chart/kube-arangodb/templates/storage-operator/role.yaml
@@ -13,9 +13,6 @@ metadata:
         app.kubernetes.io/instance: {{ .Release.Name }}
         release: {{ .Release.Name }}
 rules:
-    - apiGroups: [""]
-      resources: ["persistentvolumes", "persistentvolumeclaims", "endpoints", "events", "services"]
-      verbs: ["*"]
     - apiGroups: [""]
       resources: ["pods"]
       verbs: ["get", "update"]
diff --git a/manifests/arango-backup.yaml b/manifests/arango-backup.yaml
@@ -174,6 +174,9 @@ spec:
             hostNetwork: false
             hostPID: false
             hostIPC: false
+            securityContext:
+                runAsNonRoot: true
+                runAsUser: 1000
             containers:
                 - name: operator
                   imagePullPolicy: Always
diff --git a/manifests/arango-deployment-replication.yaml b/manifests/arango-deployment-replication.yaml
@@ -171,6 +171,9 @@ spec:
             hostNetwork: false
             hostPID: false
             hostIPC: false
+            securityContext:
+                runAsNonRoot: true
+                runAsUser: 1000
             containers:
                 - name: operator
                   imagePullPolicy: Always
diff --git a/manifests/arango-deployment.yaml b/manifests/arango-deployment.yaml
@@ -218,6 +218,9 @@ spec:
             hostNetwork: false
             hostPID: false
             hostIPC: false
+            securityContext:
+                runAsNonRoot: true
+                runAsUser: 1000
             containers:
                 - name: operator
                   imagePullPolicy: Always
diff --git a/manifests/arango-storage.yaml b/manifests/arango-storage.yaml
@@ -48,6 +48,9 @@ metadata:
         app.kubernetes.io/instance: storage
         release: storage
 rules:
+    - apiGroups: [""]
+      resources: ["persistentvolumes", "persistentvolumeclaims", "endpoints", "events", "services"]
+      verbs: ["*"]
     - apiGroups: ["apiextensions.k8s.io"]
       resources: ["customresourcedefinitions"]
       verbs: ["get"]
@@ -95,9 +98,6 @@ metadata:
         app.kubernetes.io/instance: storage
         release: storage
 rules:
-    - apiGroups: [""]
-      resources: ["persistentvolumes", "persistentvolumeclaims", "endpoints", "events", "services"]
-      verbs: ["*"]
     - apiGroups: [""]
       resources: ["pods"]
       verbs: ["get", "update"]
@@ -203,6 +203,9 @@ spec:
             hostNetwork: false
             hostPID: false
             hostIPC: false
+            securityContext:
+                runAsNonRoot: true
+                runAsUser: 1000
             containers:
                 - name: operator
                   imagePullPolicy: Always
diff --git a/pkg/backup/handlers/arango/backup/arango_client_impl.go b/pkg/backup/handlers/arango/backup/arango_client_impl.go
@@ -109,15 +109,16 @@ func (ac *arangoClientBackupImpl) Get(backupID driver.BackupID) (driver.BackupMe
 	ctx, cancel := context.WithTimeout(context.Background(), defaultArangoClientTimeout)
 	defer cancel()
 
-	list, err := ac.driver.Backup().List(ctx, &driver.BackupListOptions{ID: backupID})
+	// list, err := ac.driver.Backup().List(ctx, &driver.BackupListOptions{ID: backupID})
+	list, err := ac.driver.Backup().List(ctx, nil)
 	if err != nil {
 		return driver.BackupMeta{}, err
 	}
 
-	for id, meta := range list {
-		if id == backupID {
-			return meta, nil
-		}
+	meta, ok := list[backupID]
+
+	if ok {
+		return meta, nil
 	}
 
 	return driver.BackupMeta{}, driver.ArangoError{
@@ -201,12 +202,15 @@ func (ac *arangoClientBackupImpl) Progress(jobID driver.BackupTransferJobID) (Ar
 			completedCount++
 		case driver.TransferAcknowledged:
 		case driver.TransferStarted:
+		case "":
+			completedCount++
 		default:
 			return ArangoBackupProgress{}, fmt.Errorf("Unknown transfere status: %s", status.Status)
 		}
 	}
 
-	ret.Completed = completedCount == len(report.DBServers)
+	// Check if all defined servers are completed and total number of files is greater than 0 (there is at least 1 file per server)
+	ret.Completed = completedCount == len(report.DBServers) && total > 0
 	if total != 0 {
 		ret.Progress = (100 * done) / total
 	}
diff --git a/pkg/backup/handlers/arango/backup/arango_client_mock_test.go b/pkg/backup/handlers/arango/backup/arango_client_mock_test.go
@@ -209,14 +209,17 @@ func (m *mockArangoClientBackup) Create() (ArangoBackupCreateResponse, error) {
 		}
 	}
 
+	servers := uint(rand.Uint32())
+
 	meta := driver.BackupMeta{
 		ID:                      id,
 		Version:                 mockVersion,
-		NumberOfDBServers:       uint(rand.Uint32()),
+		NumberOfDBServers:       servers,
 		DateTime:                time.Now(),
 		SizeInBytes:             rand.Uint64(),
 		PotentiallyInconsistent: inconsistent,
 		NumberOfFiles:           uint(rand.Uint32()),
+		NumberOfPiecesPresent:   servers,
 		Available:               true,
 	}
 
diff --git a/pkg/backup/handlers/arango/backup/handler.go b/pkg/backup/handlers/arango/backup/handler.go
@@ -333,9 +333,16 @@ func (h *handler) Handle(item operation.Item) error {
 				status.State,
 				status.Message)
 		} else {
-			h.eventRecorder.Normal(b, StateChange, "Transiting from %s to %s",
-				b.Status.State,
-				status.State)
+			if status.Message != "" {
+				h.eventRecorder.Normal(b, StateChange, "Transiting from %s to %s with message: %s",
+					b.Status.State,
+					status.State,
+					status.Message)
+			} else {
+				h.eventRecorder.Normal(b, StateChange, "Transiting from %s to %s",
+					b.Status.State,
+					status.State)
+			}
 		}
 	}
 
diff --git a/pkg/backup/handlers/arango/backup/state_download.go b/pkg/backup/handlers/arango/backup/state_download.go
@@ -61,7 +61,7 @@ func stateDownloadHandler(h *handler, backup *backupApi.ArangoBackup) (*backupAp
 	if err != nil {
 		return wrapUpdateStatus(backup,
 			updateStatusState(backupApi.ArangoBackupStateDownloadError,
-				"missing field .spec.download.id"),
+				"Download failed with error: %s", err.Error()),
 			cleanStatusJob(),
 			updateStatusAvailable(false),
 		)
diff --git a/pkg/backup/handlers/arango/backup/state_ready.go b/pkg/backup/handlers/arango/backup/state_ready.go
@@ -56,7 +56,12 @@ func stateReadyHandler(h *handler, backup *backupApi.ArangoBackup) (*backupApi.A
 		)
 	}
 
-	if backupMeta.Available == false {
+	var available = backupMeta.Available
+
+	// Temporally check if number of db servers match number of pieces
+	available = available && backupMeta.NumberOfDBServers == backupMeta.NumberOfPiecesPresent
+
+	if !available {
 		return wrapUpdateStatus(backup,
 			updateStatusState(backupApi.ArangoBackupStateUnavailable, ""),
 			updateStatusBackup(backupMeta),
diff --git a/pkg/backup/handlers/arango/backup/state_ready_test.go b/pkg/backup/handlers/arango/backup/state_ready_test.go
@@ -96,6 +96,36 @@ func Test_State_Ready_Unavailable(t *testing.T) {
 	compareBackupMeta(t, backupMeta, newObj)
 }
 
+func Test_State_Ready_ServerDown(t *testing.T) {
+	// Arrange
+	handler, mock := newErrorsFakeHandler(mockErrorsArangoClientBackup{})
+
+	obj, deployment := newObjectSet(backupApi.ArangoBackupStateReady)
+
+	createResponse, err := mock.Create()
+	require.NoError(t, err)
+
+	backupMeta, err := mock.Get(createResponse.ID)
+	require.NoError(t, err)
+
+	obj.Status.Backup = createBackupFromMeta(backupMeta, nil)
+
+	backupMeta.NumberOfPiecesPresent = backupMeta.NumberOfDBServers - 1
+
+	mock.state.backups[createResponse.ID] = backupMeta
+
+	// Act
+	createArangoDeployment(t, handler, deployment)
+	createArangoBackup(t, handler, obj)
+
+	require.NoError(t, handler.Handle(newItemFromBackup(operation.Update, obj)))
+
+	// Assert
+	newObj := refreshArangoBackup(t, handler, obj)
+	checkBackup(t, newObj, backupApi.ArangoBackupStateUnavailable, false)
+	compareBackupMeta(t, backupMeta, newObj)
+}
+
 func Test_State_Ready_Success_Update(t *testing.T) {
 	// Arrange
 	handler, mock := newErrorsFakeHandler(mockErrorsArangoClientBackup{})
diff --git a/pkg/backup/handlers/arango/backup/state_unavailable.go b/pkg/backup/handlers/arango/backup/state_unavailable.go
@@ -56,7 +56,12 @@ func stateUnavailableHandler(h *handler, backup *backupApi.ArangoBackup) (*backu
 		)
 	}
 
-	if backupMeta.Available == false {
+	var available = backupMeta.Available
+
+	// Temporally check if number of db servers match number of pieces
+	available = available && backupMeta.NumberOfDBServers == backupMeta.NumberOfPiecesPresent
+
+	if !available {
 		return wrapUpdateStatus(backup,
 			updateStatusState(backupApi.ArangoBackupStateUnavailable, ""),
 			updateStatusBackup(backupMeta),
diff --git a/tests/backup_test.go b/tests/backup_test.go
@@ -324,6 +324,10 @@ func TestBackupCluster(t *testing.T) {
 	backupPolicyClient := deploymentClient.BackupV1alpha().ArangoBackupPolicies(ns)
 	backupClient := deploymentClient.BackupV1alpha().ArangoBackups(ns)
 
+	cmd := []string{
+		"--backup.api-enabled=jwt",
+	}
+
 	// Prepare deployment config
 	deplLabels := map[string]string{
 		"COMMON": "1",
@@ -333,7 +337,10 @@ func TestBackupCluster(t *testing.T) {
 	depl := newDeployment("test-backup-" + uniuri.NewLen(4))
 	depl.Spec.Mode = api.NewMode(api.DeploymentModeCluster)
 	depl.Spec.DBServers.Count = util.NewInt(2)
+	depl.Spec.DBServers.Args = cmd
 	depl.Spec.Coordinators.Count = util.NewInt(2)
+	depl.Spec.Coordinators.Args = cmd
+	depl.Spec.Agents.Args = cmd
 	depl.Spec.SetDefaults(depl.GetName()) // this must be last
 	depl.Labels = deplLabels
 	defer deferedCleanupDeployment(c, depl.GetName(), ns)
@@ -347,7 +354,10 @@ func TestBackupCluster(t *testing.T) {
 	depl2 := newDeployment("test-backup-two-" + uniuri.NewLen(4))
 	depl2.Spec.Mode = api.NewMode(api.DeploymentModeCluster)
 	depl2.Spec.DBServers.Count = util.NewInt(2)
+	depl2.Spec.DBServers.Args = cmd
 	depl2.Spec.Coordinators.Count = util.NewInt(2)
+	depl2.Spec.Coordinators.Args = cmd
+	depl2.Spec.Agents.Args = cmd
 	depl2.Spec.SetDefaults(depl2.GetName()) // this must be last
 	depl2.Labels = depl2Labels
 	defer deferedCleanupDeployment(c, depl2.GetName(), ns)

Original file line number	Diff line number	Diff line change
`@@ -209,14 +209,17 @@ func (m *mockArangoClientBackup) Create() (ArangoBackupCreateResponse, error) {`
`209`	`209`	`}`
`210`	`210`	`}`
`211`	`211`
	`212`	`+ servers := uint(rand.Uint32())`
	`213`	`+`
`212`	`214`	`meta := driver.BackupMeta{`
`213`	`215`	`ID: id,`
`214`	`216`	`Version: mockVersion,`
`215`		`- NumberOfDBServers: uint(rand.Uint32()),`
	`217`	`+ NumberOfDBServers: servers,`
`216`	`218`	`DateTime: time.Now(),`
`217`	`219`	`SizeInBytes: rand.Uint64(),`
`218`	`220`	`PotentiallyInconsistent: inconsistent,`
`219`	`221`	`NumberOfFiles: uint(rand.Uint32()),`
	`222`	`+ NumberOfPiecesPresent: servers,`
`220`	`223`	`Available: true,`
`221`	`224`	`}`
`222`	`225`