chore: refactor 1.0 parameter api #8441

sophon-zt · 2024-11-11T14:49:42Z

No description provided.

leon-inf · 2024-11-12T04:06:16Z

controllers/apps/cluster_controller.go

@@ -143,6 +143,8 @@ func (r *ClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 			&clusterServiceTransformer{},
 			// handle the restore for cluster
 			&clusterRestoreTransformer{},
+			// rerender parameters after v-scale and h-scale
+			&clusterParametersTransformer{},


It should be moved out of the cluster controller since the apps package has no knowledge about the parameters and doesn't depend on it too.

leon-inf · 2024-11-12T04:06:33Z

controllers/apps/transformer_component_reconfigure.go

+		return nil
+	}
+
+	configRender, paramsDefs, err := intctrlutil.ResolveCmpdParametersDefs(transCtx, transCtx.Client, transCtx.CompDef)


leon-inf · 2024-11-12T04:08:07Z

apis/apps/v1/componentdefinition_types.go

@@ -318,7 +318,7 @@ type ComponentDefinitionSpec struct {
 	// +listType=map
 	// +listMapKey=name
 	// +optional
-	Configs []ComponentConfigSpec `json:"configs,omitempty"`
+	Configs []ComponentTemplateSpec `json:"configs,omitempty"`


The ComponentConfigSpec could be deleted from this package?

leon-inf · 2024-11-12T04:09:44Z

apis/apps/v1/cluster_types.go

+	// Specifies the initialization parameters.
+	//
+	// +optional
+	InitParameters ComponentParameters `json:"initParameters,omitempty"`


It should be added into spec.components.configs, and be used as init values to render config templates.

codecov · 2024-11-19T10:17:36Z

Codecov Report

Attention: Patch coverage is 65.76577% with 532 lines in your changes missing coverage. Please review.

Project coverage is 59.61%. Comparing base (33bb8c8) to head (58a161a).

Files with missing lines	Patch %	Lines
controllers/parameters/policy_util.go	10.52%	85 Missing ⚠️
controllers/parameters/reconcile_task.go	69.77%	47 Missing and 21 partials ⚠️
.../parameters/componentdrivenparameter_controller.go	53.57%	53 Missing and 12 partials ⚠️
...ollers/parameters/componentparameter_controller.go	54.54%	43 Missing and 12 partials ⚠️
controllers/parameters/reconfigure_controller.go	10.52%	51 Missing ⚠️
controllers/parameters/config_util.go	51.47%	29 Missing and 4 partials ⚠️
controllers/parameters/parameter_util.go	82.55%	21 Missing and 9 partials ⚠️
...lers/parameters/parametersdefinition_controller.go	67.74%	21 Missing and 9 partials ⚠️
pkg/configuration/config_manager/handler_util.go	61.03%	23 Missing and 7 partials ⚠️
controllers/parameters/parameter_controller.go	85.07%	16 Missing and 4 partials ⚠️
... and 12 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8441      +/-   ##
==========================================
- Coverage   60.34%   59.61%   -0.73%     
==========================================
  Files         357      361       +4     
  Lines       42589    42132     -457     
==========================================
- Hits        25699    25116     -583     
- Misses      14566    14735     +169     
+ Partials     2324     2281      -43

Flag	Coverage Δ
unittests	`59.61% <65.76%> (-0.73%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚨 Try these New Features:

Flaky Tests Detection - Detect and resolve failed and flaky tests

leon-inf · 2024-11-20T05:27:53Z

controllers/apps/cluster_plan_builder.go

@@ -146,6 +147,7 @@ func init() {
 	model.AddScheme(snapshotv1beta1.AddToScheme)
 	model.AddScheme(extensionsv1alpha1.AddToScheme)
 	model.AddScheme(workloadsv1.AddToScheme)
+	model.AddScheme(parametersv1alpha1.AddToScheme)


From the perspective of their functionality and relationships, apps should no longer depend on parameters.

leon-inf · 2024-11-20T05:29:30Z

controllers/apps/transformer_cluster_deletion.go

@@ -161,6 +162,7 @@ func kindsForDelete() ([]client.ObjectList, []client.ObjectList) {
 		&corev1.SecretList{},
 		&dpv1alpha1.BackupPolicyList{},
 		&dpv1alpha1.BackupScheduleList{},
+		&parametersv1alpha1.ComponentParameterList{},


Should delete parameter resources in parameters controller.

leon-inf · 2024-11-20T05:35:06Z

controllers/apps/transformer_component_status.go

@@ -216,15 +217,15 @@ func (t *componentStatusTransformer) isAllConfigSynced(transCtx *componentTransf

 	configurationKey := client.ObjectKey{


This condition for component status should be removed, since there are no more custom resources to maintain for config/script templates.

leon-inf · 2024-11-20T05:46:32Z

controllers/apps/transformer_component_reconfigure.go

+	var configmaps []*corev1.ConfigMap
+	cachedObjs := resolveRerenderDependOnObjects(dag)
+	for _, tpls := range [][]appsv1.ComponentTemplateSpec{synthesizeComp.ScriptTemplates, synthesizeComp.ConfigTemplates} {
+		objects, err := configctrl.RenderTemplate(reconcileCtx, cluster, synthesizeComp, comp, cachedObjs, tpls)


Cloud you move the rendering out of the configuration and define it in a separate package, so that both apps and configuration can use it independently?

And, when rendering config/script templates, simply replace the variables defined by the API, no need to provide unexposed objects and built-in functions.

leon-inf · 2024-11-20T05:55:07Z

controllers/apps/transformer_component_reconfigure.go

+		return nil
+	}
+
+	return configctrl.BuildReloadActionContainer(reconcileCtx, cluster, synthesizeComp, transCtx.CompDef, configmaps)


Should be switched to the reconfigure action.

OK, this will be implemented in another pr: merge reloadAction into kbagent.

…o parameters controller

wangyelei · 2024-11-25T06:24:18Z

apis/apps/v1/component_types.go

@@ -378,3 +383,5 @@ const (
 	// FailedComponentPhase indicates that there are some pods of the component not in a 'Running' state.
 	FailedComponentPhase ComponentPhase = "Failed"
 )
+
+type ComponentParameters map[string]*string


// Better with {name:, value:}, perhaps a parameter will come from a secret or config map?

sophon-zt linked an issue Nov 11, 2024 that may be closed by this pull request

[Improvement] 1.0 config-related api #8298

Open

github-actions bot added the size/XXL Denotes a PR that changes 1000+ lines. label Nov 11, 2024

leon-inf reviewed Nov 12, 2024

View reviewed changes

sophon-zt force-pushed the support/improvement-10-config-related-api-issue8298 branch from 7455b95 to cd36433 Compare November 18, 2024 07:46

sophon-zt marked this pull request as ready for review November 18, 2024 07:47

sophon-zt requested review from ldming, wangyelei, zjx20, a team and Y-Rookie as code owners November 18, 2024 07:47

sophon-zt force-pushed the support/improvement-10-config-related-api-issue8298 branch from 6ac1501 to 115978a Compare November 20, 2024 03:33

leon-inf reviewed Nov 20, 2024

View reviewed changes

sophon-zt added 10 commits November 21, 2024 12:01

chore: add parametersdefinition

5ce7472

chore: add ParameterDrivenConfigRender

a8aa895

chore: add ComponentParameter

a3c4d28

chore: add Parameter

fec6c0f

add controller

7e7bb15

update operator for parameters

8f7a823

chore: component parameter reconcile

34842d1

update cluster controller

7edae77

chore: refine reloadAction transformer

c631e29

chore: refactor config controller

358f6b6

sophon-zt added 27 commits November 21, 2024 12:06

fix bug

adc3469

chore: adjust reconfigure ops

bead517

update crd

58575cc

fix ut

b5a7709

chore: remove ComponentTemplateSpec

d5e6feb

chore: parameter-related reconcile is migrated from apps controller t…

3b6fa81

…o parameters controller

chore: refine

74a2956

chore: refine ut

022df45

fix ut

0a32383

refine tls rerender

fa5e3d6

chore: add ut for parameters

a29766a

ut

29e535b

ut

60b1cbd

fix lint

5a12c2f

chore: rebase main

353a5fd

chore: update ut

9e0446b

chore: fix lint fails

6064ff8

fix ut fail

5c441af

chore: rename functions

34118fe

rename functions and args

a5d2001

chore: Adjusted the code according to ai suggestion

2db9e9f

chore: removed parameter-related depend from apps

952191f

split template render into separate package

97addde

add ut

d9d5027

add ut for template_render

2f8e80a

refine render package

eaa4ea0

rebase main

949fa18

sophon-zt force-pushed the support/improvement-10-config-related-api-issue8298 branch from 9c98199 to 949fa18 Compare November 21, 2024 04:38

fix lint

58a161a

wangyelei reviewed Nov 25, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: refactor 1.0 parameter api #8441

chore: refactor 1.0 parameter api #8441

sophon-zt commented Nov 11, 2024

leon-inf Nov 12, 2024

leon-inf Nov 12, 2024

leon-inf Nov 12, 2024

leon-inf Nov 12, 2024 •

edited

Loading

codecov bot commented Nov 19, 2024 •

edited

Loading

leon-inf Nov 20, 2024

leon-inf Nov 20, 2024

leon-inf Nov 20, 2024

leon-inf Nov 20, 2024

leon-inf Nov 20, 2024

leon-inf Nov 20, 2024

sophon-zt Nov 20, 2024 •

edited

Loading

wangyelei Nov 25, 2024

		@@ -216,15 +217,15 @@ func (t componentStatusTransformer) isAllConfigSynced(transCtx componentTransf

		configurationKey := client.ObjectKey{

chore: refactor 1.0 parameter api #8441

Are you sure you want to change the base?

chore: refactor 1.0 parameter api #8441

Conversation

sophon-zt commented Nov 11, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leon-inf Nov 12, 2024 • edited Loading

Choose a reason for hiding this comment

codecov bot commented Nov 19, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sophon-zt Nov 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leon-inf Nov 12, 2024 •

edited

Loading

codecov bot commented Nov 19, 2024 •

edited

Loading

sophon-zt Nov 20, 2024 •

edited

Loading