Repository: FoundationDB/fdb-kubernetes-operator
Pull request #1761 (2023-07-26 08:43:31)

Add messages from the client and cluster part in the machine-readable status
# Description

This PR adds support for the messages field in the machine-readable status. Those messages might contain important information. In this PR we only log the messages if any message is present, in follow up radars we will use this information in the operator.

## Type of change

*Please select one of the options below.*

- New feature (non-breaking change which adds functionality)

## Discussion

-

## Testing

Unit test (and e2e tests will be running).

## Documentation

-

## Follow-up

Adjust: https://github.com/FoundationDB/fdb-kubernetes-operator/pull/1758 with the new message field.


foundationdb-ci: 
### Result of fdb-kubernetes-operator-pr on Linux CentOS 7

* Commit ID: 018923459fea48fdd3405e47e089ceb3b2775fee
* Duration 4:06:37
* Result: :x: **FAILED**
* Error: `Error while executing command: if $fail_test; then exit 1; fi. Reason: exit status 1`
* [Build Log](https://d1e0l78xbkh3xa.cloudfront.net/fdb-kubernetes-operator/fdb-kubernetes-operator-pr/0a148464-173a-4118-81a2-0dbe643ae6cb.gz) terminal output (available for 30 days)
* [Build Workspace](https://d1e0l78xbkh3xa.cloudfront.net/fdb-kubernetes-operator-pr/0a148464-173a-4118-81a2-0dbe643ae6cb/workspace.zip) zip file of the working directory (available for 30 days)


Patch:
diff --git a/api/v1beta2/foundationdb_status.go b/api/v1beta2/foundationdb_status.go
index cfbcabf7..7a2744c3 100644
--- a/api/v1beta2/foundationdb_status.go
+++ b/api/v1beta2/foundationdb_status.go
@@ -42,6 +42,18 @@ type FoundationDBStatusLocalClientInfo struct {
 
 	// DatabaseStatus provides a summary of the database's health.
 	DatabaseStatus FoundationDBStatusClientDBStatus `json:"database_status,omitempty"`
+
+	// Messages represents the possible messages that are part of the client information.
+	Messages []FoundationDBStatusMessage `json:"messages,omitempty"`
+}
+
+// FoundationDBStatusMessage represents a message in the machine-readable status.
+// See: https://apple.github.io/foundationdb/mr-status.html#message-components
+type FoundationDBStatusMessage struct {
+	// Name represents the name of the message, e.g. "inconsistent_cluster_file"
+	Name string `json:"name,omitempty"`
+	// Description contains a human friendly description of the message.
+	Description string `json:"description,omitempty"`
 }
 
 // FoundationDBStatusCoordinatorInfo contains information about the client's
@@ -114,6 +126,9 @@ type FoundationDBStatusClusterInfo struct {
 
 	// ConnectionString represents the connection string in the cluster status json output.
 	ConnectionString string `json:"connection_string,omitempty"`
+
+	// Messages represents the possible messages that are part of the cluster information.
+	Messages []FoundationDBStatusMessage `json:"messages,omitempty"`
 }
 
 // FaultTolerance provides information about the fault tolerance status
diff --git a/api/v1beta2/zz_generated.deepcopy.go b/api/v1beta2/zz_generated.deepcopy.go
index df63320e..8b2c7c8a 100644
--- a/api/v1beta2/zz_generated.deepcopy.go
+++ b/api/v1beta2/zz_generated.deepcopy.go
@@ -1051,6 +1051,11 @@ func (in *FoundationDBStatusClusterInfo) DeepCopyInto(out *FoundationDBStatusClu
 		copy(*out, *in)
 	}
 	out.RecoveryState = in.RecoveryState
+	if in.Messages != nil {
+		in, out := &in.Messages, &out.Messages
+		*out = make([]FoundationDBStatusMessage, len(*in))
+		copy(*out, *in)
+	}
 }
 
 // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new FoundationDBStatusClusterInfo.
@@ -1189,6 +1194,11 @@ func (in *FoundationDBStatusLocalClientInfo) DeepCopyInto(out *FoundationDBStatu
 	*out = *in
 	in.Coordinators.DeepCopyInto(&out.Coordinators)
 	out.DatabaseStatus = in.DatabaseStatus
+	if in.Messages != nil {
+		in, out := &in.Messages, &out.Messages
+		*out = make([]FoundationDBStatusMessage, len(*in))
+		copy(*out, *in)
+	}
 }
 
 // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new FoundationDBStatusLocalClientInfo.
@@ -1216,6 +1226,21 @@ func (in *FoundationDBStatusLogInfo) DeepCopy() *FoundationDBStatusLogInfo {
 	return out
 }
 
+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *FoundationDBStatusMessage) DeepCopyInto(out *FoundationDBStatusMessage) {
+	*out = *in
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new FoundationDBStatusMessage.
+func (in *FoundationDBStatusMessage) DeepCopy() *FoundationDBStatusMessage {
+	if in == nil {
+		return nil
+	}
+	out := new(FoundationDBStatusMessage)
+	in.DeepCopyInto(out)
+	return out
+}
+
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *FoundationDBStatusMovingData) DeepCopyInto(out *FoundationDBStatusMovingData) {
 	*out = *in
diff --git a/controllers/cluster_controller.go b/controllers/cluster_controller.go
index 7e1904c4..c5974ae3 100644
--- a/controllers/cluster_controller.go
+++ b/controllers/cluster_controller.go
@@ -411,16 +411,6 @@ func (r *FoundationDBClusterReconciler) newFdbPodClient(cluster *fdbv1beta2.Foun
 	return internal.NewFdbPodClient(cluster, pod, globalControllerLogger.WithValues("namespace", cluster.Namespace, "cluster", cluster.Name, "pod", pod.Name), r.GetTimeout, r.PostTimeout)
 }
 
-func (r *FoundationDBClusterReconciler) getCoordinatorSet(cluster *fdbv1beta2.FoundationDBCluster) (map[string]fdbv1beta2.None, error) {
-	adminClient, err := r.DatabaseClientProvider.GetAdminClient(cluster, r)
-	if err != nil {
-		return map[string]fdbv1beta2.None{}, err
-	}
-	defer adminClient.Close()
-
-	return adminClient.GetCoordinatorSet()
-}
-
 // updateOrApply updates the status either with server-side apply or if disabled with the normal update call.
 func (r *FoundationDBClusterReconciler) updateOrApply(ctx context.Context, cluster *fdbv1beta2.FoundationDBCluster) error {
 	if r.ServerSideApply {
@@ -480,6 +470,14 @@ func (r *FoundationDBClusterReconciler) getStatusFromClusterOrDummyStatus(logger
 
 	status, err := adminClient.GetStatus()
 	if err == nil {
+		if len(status.Client.Messages) > 0 {
+			logger.Info("found client message(s) in the machine-readable status", "messages", status.Client.Messages)
+		}
+
+		if len(status.Cluster.Messages) > 0 {
+			logger.Info("found cluster message(s) in the machine-readable status", "messages", status.Cluster.Messages)
+		}
+
 		return status, nil
 	}
 
diff --git a/controllers/remove_process_groups.go b/controllers/remove_process_groups.go
index 1a7329fd..a45d7798 100644
--- a/controllers/remove_process_groups.go
+++ b/controllers/remove_process_groups.go
@@ -64,7 +64,8 @@ func (u removeProcessGroups) reconcile(ctx context.Context, r *FoundationDBClust
 		return &requeue{curError: err}
 	}
 
-	allExcluded, newExclusions, processGroupsToRemove := r.getProcessGroupsToRemove(logger, cluster, remainingMap)
+	coordinators := fdbstatus.GetCoordinatorsFromStatus(status)
+	allExcluded, newExclusions, processGroupsToRemove := r.getProcessGroupsToRemove(logger, cluster, remainingMap, coordinators)
 	// If no process groups are marked to remove we have to check if all process groups are excluded.
 	if len(processGroupsToRemove) == 0 {
 		if !allExcluded {
@@ -297,8 +298,7 @@ func getProcessesToInclude(cluster *fdbv1beta2.FoundationDBCluster, removedProce
 	return fdbProcessesToInclude
 }
 
-func (r *FoundationDBClusterReconciler) getProcessGroupsToRemove(logger logr.Logger, cluster *fdbv1beta2.FoundationDBCluster, remainingMap map[string]bool) (bool, bool, []*fdbv1beta2.ProcessGroupStatus) {
-	var cordSet map[string]fdbv1beta2.None
+func (r *FoundationDBClusterReconciler) getProcessGroupsToRemove(logger logr.Logger, cluster *fdbv1beta2.FoundationDBCluster, remainingMap map[string]bool, cordSet map[string]fdbv1beta2.None) (bool, bool, []*fdbv1beta2.ProcessGroupStatus) {
 	allExcluded := true
 	newExclusions := false
 	processGroupsToRemove := make([]*fdbv1beta2.ProcessGroupStatus, 0, len(cluster.Status.ProcessGroups))
@@ -308,17 +308,6 @@ func (r *FoundationDBClusterReconciler) getProcessGroupsToRemove(logger logr.Log
 			continue
 		}
 
-		// Only query FDB if we have a pending removal otherwise don't query FDB
-		if len(cordSet) == 0 {
-			var err error
-			cordSet, err = r.getCoordinatorSet(cluster)
-
-			if err != nil {
-				logger.Error(err, "Fetching coordinator set for removal")
-				return false, false, nil
-			}
-		}
-
 		if _, ok := cordSet[string(processGroup.ProcessGroupID)]; ok {
 			logger.Info("Block removal of Coordinator", "processGroupID", processGroup.ProcessGroupID)
 			allExcluded = false
diff --git a/controllers/update_status.go b/controllers/update_status.go
index 57422f1b..1a0f1938 100644
--- a/controllers/update_status.go
+++ b/controllers/update_status.go
@@ -350,6 +350,11 @@ func checkAndSetProcessStatus(logger logr.Logger, r *FoundationDBClusterReconcil
 	correct := false
 	versionCompatibleUpgrade := cluster.VersionCompatibleUpgradeInProgress()
 	for _, process := range processStatus {
+		// Check if the process is reporting any messages, those will normally include error messages
+		if len(process.Messages) > 0 {
+			logger.Info("found error message(s) for the process", "processGroupID", processGroupStatus.ProcessGroupID, "messages", process.Messages)
+		}
+
 		commandLine, err := internal.GetStartCommand(cluster, processGroupStatus.ProcessClass, podClient, processNumber, processCount)
 		if err != nil {
 			if internal.IsNetworkError(err) {
@@ -371,10 +376,12 @@ func checkAndSetProcessStatus(logger logr.Logger, r *FoundationDBClusterReconcil
 		correct = commandLine == process.CommandLine && versionMatch && !cluster.Spec.Buggify.EmptyMonitorConf
 
 		if !correct {
-			logger.Info("IncorrectProcess", "expected", commandLine, "got", process.CommandLine,
+			logger.Info("IncorrectProcess",
+				"expected", commandLine, "got", process.CommandLine,
 				"expectedVersion", cluster.Spec.Version,
-				"version", process.Version, "processGroupID", processGroupStatus.ProcessGroupID,
-				"emptyMonitorConf is ", cluster.Spec.Buggify.EmptyMonitorConf)
+				"version", process.Version,
+				"processGroupID", processGroupStatus.ProcessGroupID,
+				"emptyMonitorConf", cluster.Spec.Buggify.EmptyMonitorConf)
 		}
 	}
 


Test patch:
diff --git a/api/v1beta2/foundationdb_status_test.go b/api/v1beta2/foundationdb_status_test.go
index 9eefadfe..0105b81a 100644
--- a/api/v1beta2/foundationdb_status_test.go
+++ b/api/v1beta2/foundationdb_status_test.go
@@ -41,6 +41,7 @@ var _ = Describe("FoundationDBStatus", func() {
 			Expect(statusDecoder.Decode(&status)).NotTo(HaveOccurred())
 			Expect(status).To(Equal(FoundationDBStatus{
 				Client: FoundationDBStatusLocalClientInfo{
+					Messages: []FoundationDBStatusMessage{},
 					Coordinators: FoundationDBStatusCoordinatorInfo{
 						QuorumReachable: true,
 						Coordinators: []FoundationDBStatusCoordinator{
@@ -70,6 +71,7 @@ var _ = Describe("FoundationDBStatus", func() {
 					DatabaseStatus: FoundationDBStatusClientDBStatus{Available: true, Healthy: true},
 				},
 				Cluster: FoundationDBStatusClusterInfo{
+					Messages:                []FoundationDBStatusMessage{},
 					Generation:              62,
 					IncompatibleConnections: []string{},
 					ConnectionString:        "sample_cluster:JLjCjL6Vp3kWoIfHJeDZMhYqPBb1bIZr@10.1.38.94:4501,10.1.38.102:4501,10.1.38.104:4501",
@@ -506,6 +508,7 @@ var _ = Describe("FoundationDBStatus", func() {
 
 	When("parsing the status json with a 7.1.0-rc1 cluster", func() {
 		status := FoundationDBStatusClusterInfo{
+			Messages:                []FoundationDBStatusMessage{},
 			IncompatibleConnections: []string{},
 			ConnectionString:        "test_cluster:aHeD9ocNXOUxi0dyzU3k7Bhg53SpyrBV@10.1.18.253:4501,10.1.18.254:4501,10.1.19.0:4501",
 			DatabaseConfiguration: DatabaseConfiguration{
diff --git a/controllers/remove_process_groups_test.go b/controllers/remove_process_groups_test.go
index 0f2b2371..d2fe78dc 100644
--- a/controllers/remove_process_groups_test.go
+++ b/controllers/remove_process_groups_test.go
@@ -79,7 +79,11 @@ var _ = Describe("remove_process_groups", func() {
 					coordinatorIP: false,
 				}
 
-				allExcluded, newExclusions, processes := clusterReconciler.getProcessGroupsToRemove(globalControllerLogger, cluster, remaining)
+				coordSet := map[string]fdbv1beta2.None{
+					coordinatorIP: {},
+				}
+
+				allExcluded, newExclusions, processes := clusterReconciler.getProcessGroupsToRemove(globalControllerLogger, cluster, remaining, coordSet)
 				Expect(allExcluded).To(BeFalse())
 				Expect(processes).To(BeEmpty())
 				Expect(newExclusions).To(BeFalse())