From e65287c82a1161a0d4ee178ed6a4475058b480f1 Mon Sep 17 00:00:00 2001 From: Johannes Scheuermann Date: Fri, 26 Jul 2024 10:58:20 +0200 Subject: [PATCH] Fix the update status setup when a different image type is used (#2106) --- controllers/update_status.go | 3 +- .../operator_migate_image_type_test.go | 55 +++++++++++++++++++ internal/monitor_conf.go | 9 ++- internal/monitor_conf_test.go | 24 ++++---- pkg/fdbadminclient/mock/admin_client_mock.go | 2 +- 5 files changed, 74 insertions(+), 19 deletions(-) diff --git a/controllers/update_status.go b/controllers/update_status.go index 18074cbc..bc32db69 100644 --- a/controllers/update_status.go +++ b/controllers/update_status.go @@ -365,6 +365,7 @@ func checkAndSetProcessStatus(logger logr.Logger, r *FoundationDBClusterReconcil } versionCompatibleUpgrade := cluster.VersionCompatibleUpgradeInProgress() + imageType := internal.GetImageType(pod) for processNumber := 1; processNumber <= processCount; processNumber++ { // If the process status is present under the process group ID take that information, otherwise check if there // is information available under the process_id. @@ -392,7 +393,7 @@ func checkAndSetProcessStatus(logger logr.Logger, r *FoundationDBClusterReconcil continue } - commandLine, err := internal.GetStartCommandWithSubstitutions(cluster, processGroupStatus.ProcessClass, substitutions, processNumber, processCount) + commandLine, err := internal.GetStartCommandWithSubstitutions(cluster, processGroupStatus.ProcessClass, substitutions, processNumber, processCount, imageType) if err != nil { return err } diff --git a/e2e/test_operator_migrate_image_type/operator_migate_image_type_test.go b/e2e/test_operator_migrate_image_type/operator_migate_image_type_test.go index 794f3303..4d486db7 100644 --- a/e2e/test_operator_migrate_image_type/operator_migate_image_type_test.go +++ b/e2e/test_operator_migrate_image_type/operator_migate_image_type_test.go @@ -106,6 +106,61 @@ var _ = PDescribe("Operator Migrate Image Type", Label("e2e"), func() { }) }) + When("migrating from split to unified with Pod IP family set", func() { + BeforeEach(func() { + config := fixtures.DefaultClusterConfig(false) + config.UseUnifiedImage = pointer.Bool(false) + fdbCluster = factory.CreateFdbCluster( + config, + factory.GetClusterOptions()..., + ) + + // Set the Pod IP Family + spec := fdbCluster.GetCluster().Spec.DeepCopy() + spec.Routing.PodIPFamily = pointer.Int(4) + fdbCluster.UpdateClusterSpecWithSpec(spec) + Expect(fdbCluster.WaitForReconciliation()).NotTo(HaveOccurred()) + + // Load some data async into the cluster. We will only block as long as the Job is created. + factory.CreateDataLoaderIfAbsent(fdbCluster) + + // Update the cluster spec to run with the unified image. + spec = fdbCluster.GetCluster().Spec.DeepCopy() + imageType := fdbv1beta2.ImageTypeUnified + spec.ImageType = &imageType + // Generate the new config to make use of the unified images. + overrides := factory.GetMainContainerOverrides(false, true) + overrides.EnableTLS = spec.MainContainer.EnableTLS + spec.MainContainer = overrides + fdbCluster.UpdateClusterSpecWithSpec(spec) + Expect(fdbCluster.WaitForReconciliation()).NotTo(HaveOccurred()) + }) + + AfterEach(func() { + Expect(fdbCluster.Destroy()).NotTo(HaveOccurred()) + }) + + It("should convert the cluster", func() { + // Make sure we didn't lose data. + fdbCluster.EnsureTeamTrackersAreHealthy() + fdbCluster.EnsureTeamTrackersHaveMinReplicas() + + unifiedImage := factory.GetUnifiedFoundationDBImage() + pods := fdbCluster.GetPods() + for _, pod := range pods.Items { + // Ignore Pods that are pending the deletion. + if !pod.DeletionTimestamp.IsZero() { + continue + } + + // With the unified image no init containers are used. + Expect(pod.Spec.InitContainers).To(HaveLen(0)) + // Make sure they run the unified image. + Expect(pod.Spec.Containers[0].Image).To(ContainSubstring(unifiedImage)) + } + }) + }) + When("migrating from unified to split", func() { BeforeEach(func() { config := fixtures.DefaultClusterConfig(false) diff --git a/internal/monitor_conf.go b/internal/monitor_conf.go index 3710638f..0377a776 100644 --- a/internal/monitor_conf.go +++ b/internal/monitor_conf.go @@ -32,24 +32,22 @@ import ( ) // GetStartCommand builds the expected start command for a process group. -func GetStartCommand(cluster *fdbv1beta2.FoundationDBCluster, processClass fdbv1beta2.ProcessClass, podClient podclient.FdbPodClient, processNumber int, processCount int) (string, error) { +func GetStartCommand(cluster *fdbv1beta2.FoundationDBCluster, processClass fdbv1beta2.ProcessClass, podClient podclient.FdbPodClient, processNumber int, processCount int, imageType fdbv1beta2.ImageType) (string, error) { substitutions, err := podClient.GetVariableSubstitutions() if err != nil { return "", err } - return GetStartCommandWithSubstitutions(cluster, processClass, substitutions, processNumber, processCount) + return GetStartCommandWithSubstitutions(cluster, processClass, substitutions, processNumber, processCount, imageType) } // GetStartCommandWithSubstitutions will be used by GetStartCommand and for internal testing. -func GetStartCommandWithSubstitutions(cluster *fdbv1beta2.FoundationDBCluster, processClass fdbv1beta2.ProcessClass, substitutions map[string]string, processNumber int, processCount int) (string, error) { +func GetStartCommandWithSubstitutions(cluster *fdbv1beta2.FoundationDBCluster, processClass fdbv1beta2.ProcessClass, substitutions map[string]string, processNumber int, processCount int, imageType fdbv1beta2.ImageType) (string, error) { if substitutions == nil { return "", nil } - imageType := cluster.DesiredImageType() config := GetMonitorProcessConfiguration(cluster, processClass, processCount, imageType) - extractPlaceholderEnvVars(substitutions, config.Arguments) config.BinaryPath = fmt.Sprintf("%s/fdbserver", substitutions[fdbv1beta2.EnvNameBinaryDir]) @@ -354,5 +352,6 @@ func buildIPArgument(parameter string, environmentVariable string, imageType fdb arguments = append(arguments, monitorapi.Argument{Value: fmt.Sprintf(":%s", strings.Join(flags, ":"))}) } } + return arguments } diff --git a/internal/monitor_conf_test.go b/internal/monitor_conf_test.go index 0e5846e3..445fd880 100644 --- a/internal/monitor_conf_test.go +++ b/internal/monitor_conf_test.go @@ -489,7 +489,7 @@ var _ = Describe("monitor_conf", func() { It("should substitute the variables in the start command", func() { substitutions, err := GetSubstitutionsFromClusterAndPod(logr.Discard(), cluster, pod) Expect(err).NotTo(HaveOccurred()) - command, err = GetStartCommandWithSubstitutions(cluster, processClass, substitutions, 1, 1) + command, err = GetStartCommandWithSubstitutions(cluster, processClass, substitutions, 1, 1, cluster.DesiredImageType()) Expect(err).NotTo(HaveOccurred()) Expect(command).To(Equal(strings.Join([]string{ @@ -516,7 +516,7 @@ var _ = Describe("monitor_conf", func() { substitutions, err := GetSubstitutionsFromClusterAndPod(logr.Discard(), cluster, pod) Expect(err).NotTo(HaveOccurred()) - command, err = GetStartCommandWithSubstitutions(cluster, processClass, substitutions, 1, 1) + command, err = GetStartCommandWithSubstitutions(cluster, processClass, substitutions, 1, 1, cluster.DesiredImageType()) Expect(err).NotTo(HaveOccurred()) Expect(command).To(Equal(strings.Join([]string{ @@ -540,7 +540,7 @@ var _ = Describe("monitor_conf", func() { It("should substitute the variables in the start command", func() { substitutions, err := GetSubstitutionsFromClusterAndPod(logr.Discard(), cluster, pod) Expect(err).NotTo(HaveOccurred()) - command, err = GetStartCommandWithSubstitutions(cluster, processClass, substitutions, 1, 2) + command, err = GetStartCommandWithSubstitutions(cluster, processClass, substitutions, 1, 2, cluster.DesiredImageType()) Expect(err).NotTo(HaveOccurred()) id := "storage-1" @@ -559,7 +559,7 @@ var _ = Describe("monitor_conf", func() { "--seed_cluster_file=/var/dynamic-conf/fdb.cluster", }, " "))) - command, err = GetStartCommandWithSubstitutions(cluster, processClass, substitutions, 2, 2) + command, err = GetStartCommandWithSubstitutions(cluster, processClass, substitutions, 2, 2, cluster.DesiredImageType()) Expect(err).NotTo(HaveOccurred()) Expect(command).To(Equal(strings.Join([]string{ "/usr/bin/fdbserver", @@ -585,7 +585,7 @@ var _ = Describe("monitor_conf", func() { substitutions, err := GetSubstitutionsFromClusterAndPod(logr.Discard(), cluster, pod) Expect(err).NotTo(HaveOccurred()) - command, err = GetStartCommandWithSubstitutions(cluster, fdbv1beta2.ProcessClassStorage, substitutions, 1, 1) + command, err = GetStartCommandWithSubstitutions(cluster, fdbv1beta2.ProcessClassStorage, substitutions, 1, 1, cluster.DesiredImageType()) Expect(err).NotTo(HaveOccurred()) }) @@ -617,7 +617,7 @@ var _ = Describe("monitor_conf", func() { substitutions, err := GetSubstitutionsFromClusterAndPod(logr.Discard(), cluster, pod) Expect(err).NotTo(HaveOccurred()) - command, err = GetStartCommandWithSubstitutions(cluster, fdbv1beta2.ProcessClassStorage, substitutions, 1, 1) + command, err = GetStartCommandWithSubstitutions(cluster, fdbv1beta2.ProcessClassStorage, substitutions, 1, 1, cluster.DesiredImageType()) Expect(err).NotTo(HaveOccurred()) }) @@ -644,7 +644,7 @@ var _ = Describe("monitor_conf", func() { cluster.Status.RunningVersion = fdbv1beta2.Versions.Default.String() substitutions, err := GetSubstitutionsFromClusterAndPod(logr.Discard(), cluster, pod) Expect(err).NotTo(HaveOccurred()) - command, err = GetStartCommandWithSubstitutions(cluster, fdbv1beta2.ProcessClassStorage, substitutions, 1, 1) + command, err = GetStartCommandWithSubstitutions(cluster, fdbv1beta2.ProcessClassStorage, substitutions, 1, 1, cluster.DesiredImageType()) Expect(err).NotTo(HaveOccurred()) }) @@ -676,7 +676,7 @@ var _ = Describe("monitor_conf", func() { It("should generate the unsorted command-line", func() { substitutions, err := GetSubstitutionsFromClusterAndPod(logr.Discard(), cluster, pod) Expect(err).NotTo(HaveOccurred()) - command, err = GetStartCommandWithSubstitutions(cluster, processClass, substitutions, 1, 1) + command, err = GetStartCommandWithSubstitutions(cluster, processClass, substitutions, 1, 1, cluster.DesiredImageType()) Expect(err).NotTo(HaveOccurred()) Expect(command).To(Equal(strings.Join([]string{ @@ -699,7 +699,7 @@ var _ = Describe("monitor_conf", func() { It("should fill in the process number", func() { substitutions, err := GetSubstitutionsFromClusterAndPod(logr.Discard(), cluster, pod) Expect(err).NotTo(HaveOccurred()) - command, err = GetStartCommandWithSubstitutions(cluster, processClass, substitutions, 2, 3) + command, err = GetStartCommandWithSubstitutions(cluster, processClass, substitutions, 2, 3, cluster.DesiredImageType()) Expect(err).NotTo(HaveOccurred()) Expect(command).To(Equal(strings.Join([]string{ @@ -732,7 +732,7 @@ var _ = Describe("monitor_conf", func() { It("should substitute the variables in the custom parameters", func() { substitutions, err := GetSubstitutionsFromClusterAndPod(logr.Discard(), cluster, pod) Expect(err).NotTo(HaveOccurred()) - command, err = GetStartCommandWithSubstitutions(cluster, processClass, substitutions, 1, 1) + command, err = GetStartCommandWithSubstitutions(cluster, processClass, substitutions, 1, 1, cluster.DesiredImageType()) Expect(err).NotTo(HaveOccurred()) Expect(command).To(Equal(strings.Join([]string{ "/usr/bin/fdbserver", @@ -766,7 +766,7 @@ var _ = Describe("monitor_conf", func() { It("should substitute the variables in the custom parameters", func() { substitutions, err := GetSubstitutionsFromClusterAndPod(logr.Discard(), cluster, pod) Expect(err).NotTo(HaveOccurred()) - command, err = GetStartCommandWithSubstitutions(cluster, processClass, substitutions, 1, 1) + command, err = GetStartCommandWithSubstitutions(cluster, processClass, substitutions, 1, 1, cluster.DesiredImageType()) Expect(err).NotTo(HaveOccurred()) Expect(command).To(Equal(strings.Join([]string{ "/usr/bin/fdbserver", @@ -799,7 +799,7 @@ var _ = Describe("monitor_conf", func() { It("should substitute the variables in the custom parameters", func() { substitutions, err := GetSubstitutionsFromClusterAndPod(logr.Discard(), cluster, pod) Expect(err).NotTo(HaveOccurred()) - command, err = GetStartCommandWithSubstitutions(cluster, processClass, substitutions, 1, 1) + command, err = GetStartCommandWithSubstitutions(cluster, processClass, substitutions, 1, 1, cluster.DesiredImageType()) Expect(err).NotTo(HaveOccurred()) Expect(command).To(Equal(strings.Join([]string{ "/usr/bin/fdbserver", diff --git a/pkg/fdbadminclient/mock/admin_client_mock.go b/pkg/fdbadminclient/mock/admin_client_mock.go index dcd9c332..68f2b358 100644 --- a/pkg/fdbadminclient/mock/admin_client_mock.go +++ b/pkg/fdbadminclient/mock/admin_client_mock.go @@ -200,7 +200,7 @@ func (client *AdminClient) GetStatus() (*fdbv1beta2.FoundationDBStatus, error) { command, hasCommandLine := client.currentCommandLines[fullAddress.StringWithoutFlags()] if !hasCommandLine { // We only set the command if we don't have the commandline "cached" - command, err = internal.GetStartCommand(client.Cluster, pClass, podClient, processIndex, processCount) + command, err = internal.GetStartCommand(client.Cluster, pClass, podClient, processIndex, processCount, internal.GetImageType(&pod)) if err != nil { return nil, err }