Update the client compatibility check to support cases where the clients have a newer version (#2320)

* Update the client compatibility check to support cases where the clients have a newer version

Author: johscheuer

api/v1beta2/foundationdbcluster_types.go

@@ -1902,7 +1902,14 @@ func (cluster *FoundationDBCluster) CheckReconciliation(log logr.Logger) (bool,
 		}
 	}
 
-	if reconciled {
+	if reconciled && cluster.Status.Generations.Reconciled != cluster.Generation {
+		logger.Info(
+			"Update reconciled generation",
+			"previousGeneration",
+			cluster.Status.Generations.Reconciled,
+			"currentGeneration",
+			cluster.Generation,
+		)
 		cluster.Status.Generations.Reconciled = cluster.Generation
 	}
 

controllers/check_client_compatibility.go

@@ -117,46 +117,68 @@ func (c checkClientCompatibility) reconcile(
 	return nil
 }
 
+// foundationDBClient represents a client of the FoundationDBCluster
+type foundationDBClient struct {
+	// description represents the client description.
+	description string
+	// supportedVersions is a map of all supported versions of this client.
+	supportedVersions map[string]fdbv1beta2.None
+}
+
 func getUnsupportedClients(
 	status *fdbv1beta2.FoundationDBStatus,
 	protocolVersion string,
 	ignoredLogGroups map[fdbv1beta2.LogGroup]fdbv1beta2.None,
 ) []string {
-	var unsupportedClients []string
-
 	processAddresses := map[string]fdbv1beta2.None{}
 	for _, process := range status.Cluster.Processes {
 		processAddresses[process.Address.MachineAddress()] = fdbv1beta2.None{}
 	}
 
+	var unsupportedClients []string
+	clientSupportedVersions := map[string]*foundationDBClient{}
 	for _, versionInfo := range status.Cluster.Clients.SupportedVersions {
 		if versionInfo.ProtocolVersion == "Unknown" {
 			continue
 		}
 
-		if versionInfo.ProtocolVersion != protocolVersion {
-			for _, client := range versionInfo.MaxProtocolClients {
-				if _, ok := ignoredLogGroups[client.LogGroup]; ok {
-					continue
-				}
+		for _, client := range versionInfo.ConnectedClients {
+			if _, ok := ignoredLogGroups[client.LogGroup]; ok {
+				continue
+			}
 
-				addr, err := fdbv1beta2.ParseProcessAddress(client.Address)
-				// In case we are not able to parse the address, we assume it is an unsupported client.
-				if err != nil {
-					unsupportedClients = append(unsupportedClients, client.Description())
-					continue
-				}
+			addr, err := fdbv1beta2.ParseProcessAddress(client.Address)
+			// In case we are not able to parse the address, we assume it is an unsupported client.
+			if err != nil {
+				unsupportedClients = append(unsupportedClients, client.Description())
+				continue
+			}
 
-				// If the address is from a running process, it's probably something running in one of the FoundationDB
-				// Pods, like someone manually ran `fdbcli`.
-				if _, ok := processAddresses[addr.MachineAddress()]; ok {
-					continue
-				}
+			// If the address is from a running process, it's probably something running in one of the FoundationDB
+			// Pods, like someone manually ran `fdbcli`.
+			if _, ok := processAddresses[addr.MachineAddress()]; ok {
+				continue
+			}
 
-				unsupportedClients = append(unsupportedClients, client.Description())
+			if currentClient, ok := clientSupportedVersions[addr.String()]; ok {
+				currentClient.supportedVersions[versionInfo.ProtocolVersion] = fdbv1beta2.None{}
+			} else {
+				clientSupportedVersions[addr.String()] = &foundationDBClient{
+					description: client.Description(),
+					supportedVersions: map[string]fdbv1beta2.None{
+						versionInfo.ProtocolVersion: {},
+					},
+				}
 			}
 		}
 	}
 
+	// Validate for all clients that they support the requested version.
+	for _, client := range clientSupportedVersions {
+		if _, ok := client.supportedVersions[protocolVersion]; !ok {
+			unsupportedClients = append(unsupportedClients, client.description)
+		}
+	}
+
 	return unsupportedClients
 }

controllers/check_client_compatibility_test.go

@@ -348,5 +348,112 @@ var _ = Describe("check client compatibility", func() {
 				})
 			})
 		})
+
+		When("the status contains newer versions and the desired version is present", func() {
+			BeforeEach(func() {
+				status = &fdbv1beta2.FoundationDBStatus{
+					Cluster: fdbv1beta2.FoundationDBStatusClusterInfo{
+						Clients: fdbv1beta2.FoundationDBStatusClusterClientInfo{
+							SupportedVersions: []fdbv1beta2.FoundationDBStatusSupportedVersion{
+								{
+									ClientVersion: "Unknown",
+									ConnectedClients: []fdbv1beta2.FoundationDBStatusConnectedClient{
+										{
+											Address:  "10.1.38.92:52762",
+											LogGroup: "default",
+										},
+										{
+											Address:  "10.1.38.103:43346",
+											LogGroup: "default",
+										},
+									},
+									MaxProtocolClients: nil,
+									ProtocolVersion:    "Unknown",
+									SourceVersion:      "Unknown",
+								},
+								{
+									ClientVersion: "6.1.8",
+									ConnectedClients: []fdbv1beta2.FoundationDBStatusConnectedClient{
+										{
+											Address:  "10.1.38.92:52762",
+											LogGroup: "default",
+										},
+										{
+											Address:  "10.1.38.103:43346",
+											LogGroup: "default",
+										},
+									},
+									MaxProtocolClients: nil,
+									ProtocolVersion:    "fdb00b061060001",
+									SourceVersion:      "bd6b10cbcee08910667194e6388733acd3b80549",
+								},
+								{
+									ClientVersion: "6.2.15",
+									ConnectedClients: []fdbv1beta2.FoundationDBStatusConnectedClient{
+										{
+											Address:  "10.1.38.92:52762",
+											LogGroup: "default",
+										},
+										{
+											Address:  "10.1.38.103:43346",
+											LogGroup: "default",
+										},
+									},
+									MaxProtocolClients: nil,
+									ProtocolVersion:    "fdb00b062010002",
+									SourceVersion:      "20566f2ff06a7e822b30e8cfd91090fbd863a393",
+								},
+
+								{
+									ClientVersion: "6.3.15",
+									ConnectedClients: []fdbv1beta2.FoundationDBStatusConnectedClient{
+										{
+											Address:  "10.1.38.92:52762",
+											LogGroup: "default",
+										},
+										{
+											Address:  "10.1.38.103:43346",
+											LogGroup: "default",
+										},
+									},
+									MaxProtocolClients: nil,
+									ProtocolVersion:    "fdb00b063010001",
+									SourceVersion:      "20566f2ff06a7e822b30e8cfd91090fbd863a393",
+								},
+								{
+									ClientVersion: "7.4.2",
+									ConnectedClients: []fdbv1beta2.FoundationDBStatusConnectedClient{
+										{
+											Address:  "10.1.38.92:52762",
+											LogGroup: "default",
+										},
+										{
+											Address:  "10.1.38.103:43346",
+											LogGroup: "default",
+										},
+									},
+									MaxProtocolClients: []fdbv1beta2.FoundationDBStatusConnectedClient{
+										{
+											Address:  "10.1.38.92:52762",
+											LogGroup: "default",
+										},
+										{
+											Address:  "10.1.38.103:43346",
+											LogGroup: "default",
+										},
+									},
+									ProtocolVersion: "fdb00b074000000",
+									SourceVersion:   "eb4d3fe30ecf8481e1ab966b7ef6345eb66efcf8",
+								},
+							},
+						},
+					},
+				}
+			})
+
+			It("should detect everything correctly", func() {
+				Expect(unsupportedClients).To(BeEmpty())
+			})
+		})
 	})
 })

controllers/cluster_controller_test.go

@@ -2969,9 +2969,7 @@ var _ = Describe("cluster_controller", func() {
 
 				It("should set the image on the pods", func() {
 					pods := &corev1.PodList{}
-					err = k8sClient.List(context.TODO(), pods, getListOptions(cluster)...)
-					Expect(err).NotTo(HaveOccurred())
-
+					Expect(k8sClient.List(context.TODO(), pods)).To(Succeed())
 					for _, pod := range pods.Items {
 						Expect(
 							pod.Spec.Containers[0].Image,
@@ -2995,17 +2993,14 @@ var _ = Describe("cluster_controller", func() {
 					events := &corev1.EventList{}
 					var matchingEvents []corev1.Event
 
-					err = k8sClient.List(context.TODO(), events)
-					Expect(err).NotTo(HaveOccurred())
-
+					Expect(k8sClient.List(context.TODO(), events)).To(Succeed())
 					for _, event := range events.Items {
 						if event.InvolvedObject.UID == cluster.ObjectMeta.UID &&
 							event.Reason == "UnsupportedClient" {
 							matchingEvents = append(matchingEvents, event)
 						}
 					}
 					Expect(len(matchingEvents)).NotTo(Equal(0))
-
 					Expect(matchingEvents[0].Message).To(Equal(
 						fmt.Sprintf(
 							"1 clients do not support version 7.2.0: 127.0.0.2:3687 (%s)",

pkg/fdbadminclient/mock/admin_client_mock.go

@@ -417,6 +417,7 @@ func (client *AdminClient) GetStatus() (*fdbv1beta2.FoundationDBStatus, error) {
 				fdbv1beta2.FoundationDBStatusSupportedVersion{
 					ClientVersion:      version,
 					ProtocolVersion:    protocolVersion,
+					ConnectedClients:   protocolClients,
 					MaxProtocolClients: protocolClients,
 				},
 			)