Skip to content

Commit

Permalink
Fix the update status setup when a different image type is used
Browse files Browse the repository at this point in the history
  • Loading branch information
@johscheuer
johscheuer committed Jul 24, 2024
1 parent 19b81c7 commit 08af871
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 19 deletions.
3 changes: 2 additions & 1 deletion controllers/update_status.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
}
Expand Down
55 changes: 55 additions & 0 deletions e2e/test_operator_migrate_image_type/operator_migate_image_type_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
9 changes: 4 additions & 5 deletions internal/monitor_conf.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand Down Expand Up @@ -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
}
24 changes: 12 additions & 12 deletions internal/monitor_conf_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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{
Expand All @@ -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"
Expand All @@ -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",
Expand All @@ -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())
})

Expand Down Expand Up @@ -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())
})

Expand All @@ -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())
})

Expand Down Expand Up @@ -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{
Expand All @@ -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{
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion pkg/fdbadminclient/mock/admin_client_mock.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down

0 comments on commit 08af871

Please sign in to comment.