fix: compoents don't updated by modifying the fields of cluster.base (

#192) * fix: compoents don't updated by modifying the fields of `cluster.base` Signed-off-by: zyy17 <[email protected]> * chore: move 'greptimedbcluster/test03' directory * test: add unit test for merge * refactor: set spec.Version by base config * refactor: make meta/frontend/datanode component optional --------- Signed-off-by: zyy17 <[email protected]>
GreptimeTeam · Oct 15, 2024 · a236c37 · a236c37
1 parent 4b14b1c
commit a236c37
Show file tree

Hide file tree

Showing 17 changed files with 315 additions and 256 deletions.
diff --git a/apis/v1alpha1/defaulting.go b/apis/v1alpha1/defaulting.go
@@ -25,31 +25,77 @@ import (
 	"k8s.io/utils/pointer"
 )
 
+// SetDefaults sets the default values for the GreptimeDBCluster.
 func (in *GreptimeDBCluster) SetDefaults() error {
 	if in == nil {
 		return nil
 	}
 
 	// Set the version of the GreptimeDBClusterSpec if it is not set.
-	if in.GetVersion() == "" && in.GetBaseMainContainer().GetImage() != "" {
-		in.Spec.Version = getVersionFromImage(in.GetBaseMainContainer().GetImage())
-	}
+	in.Spec.Version = getVersionFromImage(in.GetBaseMainContainer().GetImage())
 
 	// Merge the default settings into the GreptimeDBClusterSpec.
 	if err := mergo.Merge(&in.Spec, in.defaultSpec()); err != nil {
 		return err
 	}
 
-	// Merge the Base field into the frontend/meta/datanode/flownode template.
-	if err := in.mergeTemplate(); err != nil {
-		return err
+	return nil
+}
+
+// MergeTemplate merges the base template with the component's template.
+func (in *GreptimeDBCluster) MergeTemplate() error {
+	mergeFuncs := []func() error{
+		in.mergeFrontendTemplate,
+		in.mergeMetaTemplate,
+		in.mergeDatanodeTemplate,
+		in.mergeFlownodeTemplate,
+	}
+
+	for _, mergeFunc := range mergeFuncs {
+		if err := mergeFunc(); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+// MergeLogging merges the logging settings into the component's logging settings.
+func (in *GreptimeDBCluster) MergeLogging() error {
+	loggingSpecs := []*LoggingSpec{
+		in.GetMeta().GetLogging(),
+		in.GetDatanode().GetLogging(),
+		in.GetFrontend().GetLogging(),
+		in.GetFlownode().GetLogging(),
+	}
+
+	for _, logging := range loggingSpecs {
+		if logging == nil {
+			continue
+		}
+
+		if err := in.doMergeLogging(logging, in.GetLogging(), in.GetMonitoring().IsEnabled()); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+func (in *GreptimeDBCluster) doMergeLogging(input, global *LoggingSpec, isEnableMonitoring bool) error {
+	if input == nil || global == nil {
+		return nil
 	}
 
-	// Merge the logging settings into the GreptimeDBClusterSpec.
-	if err := in.mergeLogging(); err != nil {
+	if err := mergo.Merge(input, global.DeepCopy()); err != nil {
 		return err
 	}
 
+	if isEnableMonitoring {
+		// Set the default logging format to JSON if monitoring is enabled.
+		input.Format = LogFormatJSON
+	}
+
 	return nil
 }
 
@@ -200,26 +246,6 @@ func (in *GreptimeDBCluster) defaultMonitoringStandaloneSpec() *GreptimeDBStanda
 	return &standalone.Spec
 }
 
-func (in *GreptimeDBCluster) mergeTemplate() error {
-	if err := in.mergeFrontendTemplate(); err != nil {
-		return err
-	}
-
-	if err := in.mergeMetaTemplate(); err != nil {
-		return err
-	}
-
-	if err := in.mergeDatanodeTemplate(); err != nil {
-		return err
-	}
-
-	if err := in.mergeFlownodeTemplate(); err != nil {
-		return err
-	}
-
-	return nil
-}
-
 func (in *GreptimeDBCluster) mergeFrontendTemplate() error {
 	if in.Spec.Frontend != nil {
 		// Use DeepCopy to avoid the same pointer.
@@ -276,58 +302,12 @@ func (in *GreptimeDBCluster) mergeFlownodeTemplate() error {
 	return nil
 }
 
-func (in *GreptimeDBCluster) mergeLogging() error {
-	if logging := in.GetMeta().GetLogging(); logging != nil {
-		if err := mergo.Merge(logging, in.GetLogging().DeepCopy()); err != nil {
-			return err
-		}
-		if in.GetMonitoring().IsEnabled() {
-			// Set the default logging format to JSON if monitoring is enabled.
-			logging.Format = LogFormatJSON
-		}
-	}
-
-	if logging := in.GetDatanode().GetLogging(); logging != nil {
-		if err := mergo.Merge(logging, in.GetLogging().DeepCopy()); err != nil {
-			return err
-		}
-		if in.GetMonitoring().IsEnabled() {
-			// Set the default logging format to JSON if monitoring is enabled.
-			logging.Format = LogFormatJSON
-		}
-	}
-
-	if logging := in.GetFrontend().GetLogging(); logging != nil {
-		if err := mergo.Merge(logging, in.GetLogging().DeepCopy()); err != nil {
-			return err
-		}
-		if in.GetMonitoring().IsEnabled() {
-			// Set the default logging format to JSON if monitoring is enabled.
-			logging.Format = LogFormatJSON
-		}
-	}
-
-	if logging := in.GetFlownode().GetLogging(); logging != nil {
-		if err := mergo.Merge(logging, in.GetLogging().DeepCopy()); err != nil {
-			return err
-		}
-		if in.GetMonitoring().IsEnabled() {
-			// Set the default logging format to JSON if monitoring is enabled.
-			logging.Format = LogFormatJSON
-		}
-	}
-
-	return nil
-}
-
 func (in *GreptimeDBStandalone) SetDefaults() error {
 	if in == nil {
 		return nil
 	}
 
-	if in.GetVersion() == "" && in.GetBaseMainContainer().GetImage() != "" {
-		in.Spec.Version = getVersionFromImage(in.GetBaseMainContainer().GetImage())
-	}
+	in.Spec.Version = getVersionFromImage(in.GetBaseMainContainer().GetImage())
 
 	if err := mergo.Merge(&in.Spec, in.defaultSpec()); err != nil {
 		return err

diff --git a/apis/v1alpha1/defaulting_test.go b/apis/v1alpha1/defaulting_test.go
@@ -26,7 +26,7 @@ import (
 
 func TestClusterSetDefaults(t *testing.T) {
 	const (
-		testDir        = "testdata/defaulting/greptimedbcluster"
+		testDir        = "testdata/defaulting/greptimedbcluster/setdefaults"
 		inputFileName  = "input.yaml"
 		expectFileName = "expect.yaml"
 	)
@@ -84,6 +84,74 @@ func TestClusterSetDefaults(t *testing.T) {
 	}
 }
 
+func TestClusterMerge(t *testing.T) {
+	const (
+		testDir        = "testdata/defaulting/greptimedbcluster/merge"
+		inputFileName  = "input.yaml"
+		expectFileName = "expect.yaml"
+	)
+
+	entries, err := os.ReadDir(testDir)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	for _, entry := range entries {
+		if entry.IsDir() {
+			inputFile := filepath.Join(testDir, entry.Name(), inputFileName)
+			inputData, err := os.ReadFile(inputFile)
+			if err != nil {
+				t.Errorf("failed to read %s: %v", inputFile, err)
+			}
+
+			expectFile := filepath.Join(testDir, entry.Name(), expectFileName)
+			expectData, err := os.ReadFile(expectFile)
+			if err != nil {
+				t.Fatalf("failed to read %s: %v", expectFile, err)
+			}
+
+			var (
+				input  GreptimeDBCluster
+				expect GreptimeDBCluster
+			)
+			if err := yaml.Unmarshal(inputData, &input); err != nil {
+				t.Fatalf("failed to unmarshal %s: %v", inputFile, err)
+			}
+			if err := yaml.Unmarshal(expectData, &expect); err != nil {
+				t.Fatalf("failed to unmarshal %s: %v", expectFile, err)
+			}
+
+			if err := input.SetDefaults(); err != nil {
+				t.Fatalf("failed to set defaults: %v", err)
+			}
+
+			if err := input.MergeTemplate(); err != nil {
+				t.Fatalf("failed to merge template: %v", err)
+			}
+
+			if err := input.MergeLogging(); err != nil {
+				t.Fatalf("failed to merge logging: %v", err)
+			}
+
+			if !reflect.DeepEqual(input, expect) {
+				rawInputData, err := yaml.Marshal(input)
+				if err != nil {
+					t.Fatalf("failed to marshal: %v", err)
+				}
+
+				rawExpectData, err := yaml.Marshal(expect)
+				if err != nil {
+					t.Fatalf("failed to marshal: %v", err)
+				}
+
+				// Use diffmatchpatch to get a human-readable diff.
+				dmp := diffmatchpatch.New()
+				t.Errorf("unexpected result for %s:\n%s", entry.Name(), dmp.DiffPrettyText(dmp.DiffMain(string(rawExpectData), string(rawInputData), false)))
+			}
+		}
+	}
+}
+
 func TestStandaloneSetDefaults(t *testing.T) {
 	const (
 		testDir        = "testdata/defaulting/greptimedbstandalone"

diff --git a/apis/v1alpha1/greptimedbcluster_types.go b/apis/v1alpha1/greptimedbcluster_types.go
@@ -261,16 +261,16 @@ type GreptimeDBClusterSpec struct {
 	Base *PodTemplateSpec `json:"base,omitempty"`
 
 	// Frontend is the specification of frontend node.
-	// +required
-	Frontend *FrontendSpec `json:"frontend"`
+	// +optional
+	Frontend *FrontendSpec `json:"frontend,omitempty"`
 
 	// Meta is the specification of meta node.
-	// +required
-	Meta *MetaSpec `json:"meta"`
+	// +optional
+	Meta *MetaSpec `json:"meta,omitempty"`
 
 	// Datanode is the specification of datanode node.
-	// +required
-	Datanode *DatanodeSpec `json:"datanode"`
+	// +optional
+	Datanode *DatanodeSpec `json:"datanode,omitempty"`
 
 	// Flownode is the specification of flownode node.
 	// +optional