From b4de54aead12cb2355ae97efc18e08727e7c1d85 Mon Sep 17 00:00:00 2001 From: Wei Cao Date: Mon, 6 Jul 2026 02:43:45 +0800 Subject: [PATCH] fix(operations): fail reconfigure fast when the component does not support parameters A Reconfigure OpsRequest targeting a component whose ComponentParameter CR does not exist retried forever on a bare NotFound error, on both the Action and ReconcileAction paths, because only fatal errors transition an OpsRequest to Failed. Classify the NotFound with the same predicate the componentdrivenparameter controller uses to decide whether it will ever create the ComponentParameter (ComponentDefinition config templates resolving to valid ParametersDefinitions): if the component provably does not support parameters, fail fast with a clear message; otherwise keep the existing retry behavior to tolerate creation lag. Fixes #10545 --- pkg/operations/reconfigure.go | 42 +++++++++ pkg/operations/reconfigure_test.go | 133 +++++++++++++++++++++++++++++ 2 files changed, 175 insertions(+) diff --git a/pkg/operations/reconfigure.go b/pkg/operations/reconfigure.go index 06b6814d22b..728649de019 100644 --- a/pkg/operations/reconfigure.go +++ b/pkg/operations/reconfigure.go @@ -24,15 +24,18 @@ import ( "fmt" "time" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" appsv1 "github.com/apecloud/kubeblocks/apis/apps/v1" opsv1alpha1 "github.com/apecloud/kubeblocks/apis/operations/v1alpha1" parametersv1alpha1 "github.com/apecloud/kubeblocks/apis/parameters/v1alpha1" + "github.com/apecloud/kubeblocks/pkg/constant" "github.com/apecloud/kubeblocks/pkg/controller/component" "github.com/apecloud/kubeblocks/pkg/controller/sharding" intctrlutil "github.com/apecloud/kubeblocks/pkg/controllerutil" + "github.com/apecloud/kubeblocks/pkg/parameters" parameterscore "github.com/apecloud/kubeblocks/pkg/parameters/core" ) @@ -185,7 +188,46 @@ func (r *reconfigureAction) getRunningComponentParameter(ctx context.Context, cl Name: parameterscore.GenerateComponentConfigurationName(clusterName, compName), } if err := cli.Get(ctx, key, compParam); err != nil { + if apierrors.IsNotFound(err) { + return nil, r.classifyComponentParameterNotFound(ctx, cli, namespace, clusterName, compName, err) + } return nil, err } return compParam, nil } + +// classifyComponentParameterNotFound classifies a missing ComponentParameter object. +// The componentdrivenparameter controller creates the ComponentParameter only when the +// component's ComponentDefinition declares config templates that resolve to valid +// ParametersDefinitions. If they don't, the object will never appear and retrying is +// pointless, so fail the operation fast. In any other case (e.g. the controller has not +// caught up yet, or the support status cannot be proven), return the original NotFound +// error to keep the existing retry behavior. +func (r *reconfigureAction) classifyComponentParameterNotFound(ctx context.Context, cli client.Client, namespace, clusterName, compName string, notFoundErr error) error { + comp := &appsv1.Component{} + compKey := client.ObjectKey{ + Namespace: namespace, + Name: constant.GenerateClusterComponentName(clusterName, compName), + } + if err := cli.Get(ctx, compKey, comp); err != nil { + return notFoundErr + } + cmpd := &appsv1.ComponentDefinition{} + if err := cli.Get(ctx, client.ObjectKey{Name: comp.Spec.CompDef}, cmpd); err != nil { + return notFoundErr + } + if len(cmpd.Spec.Configs) != 0 { + configDescs, _, err := parameters.ResolveCmpdParametersDefs(ctx, cli, cmpd) + if err != nil { + // cannot prove that the component does not support parameters, keep retrying. + return notFoundErr + } + if parameters.HasValidParameterTemplate(configDescs) { + // the component supports parameters, the ComponentParameter object has not + // been created by the componentdrivenparameter controller yet, keep retrying. + return notFoundErr + } + } + return intctrlutil.NewErrorf(intctrlutil.ErrorTypeFatal, + "component %s does not support reconfigure: ComponentParameter not found", compName) +} diff --git a/pkg/operations/reconfigure_test.go b/pkg/operations/reconfigure_test.go index 4b055a808b7..aaf83fc2146 100644 --- a/pkg/operations/reconfigure_test.go +++ b/pkg/operations/reconfigure_test.go @@ -24,6 +24,7 @@ import ( . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/utils/pointer" "sigs.k8s.io/controller-runtime/pkg/client" @@ -68,6 +69,7 @@ var _ = Describe("Reconfigure OpsRequest", func() { testapps.ClearResources(&testCtx, generics.ParametersDefinitionSignature, ml) testapps.ClearResourcesWithRemoveFinalizerOption(&testCtx, generics.InstanceSetSignature, true, inNS, ml) testapps.ClearResourcesWithRemoveFinalizerOption(&testCtx, generics.ComponentParameterSignature, true, inNS) + testapps.ClearResourcesWithRemoveFinalizerOption(&testCtx, generics.ComponentSignature, true, inNS, ml) } BeforeEach(cleanEnv) @@ -264,5 +266,136 @@ parameter: { g.Expect(condition.Message).Should(ContainSubstring("maxmemory-samples")) })).Should(Succeed()) }) + + newReconfigureOps := func(opsName string) *opsv1alpha1.OpsRequest { + ops := testops.NewOpsRequestObj(opsName, testCtx.DefaultNamespace, + clusterName, opsv1alpha1.ReconfiguringType) + ops.Spec.Reconfigures = []opsv1alpha1.Reconfigure{ + { + ComponentOps: opsv1alpha1.ComponentOps{ComponentName: defaultCompName}, + Parameters: []opsv1alpha1.ParameterPair{ + { + Key: "max_connections", + Value: pointer.String("200"), + }, + }, + }, + } + return ops + } + + createComponentObject := func() { + testapps.NewComponentFactory(testCtx.DefaultNamespace, + constant.GenerateClusterComponentName(clusterName, defaultCompName), compDefName). + Create(&testCtx) + } + + prepareParameterSupport := func() { + template := testparameters.NewComponentTemplateFactory("mysql-config", testCtx.DefaultNamespace). + Create(&testCtx). + GetObject() + paramsDef := testparameters.NewParametersDefinitionFactory("mysql-params-" + randomStr). + SetComponentDefinition(compDefName). + SetTemplateName("mysql-config"). + Schema(` +parameter: { + max_connections?: string +}`). + Create(&testCtx). + GetObject() + Expect(testapps.ChangeObjStatus(&testCtx, paramsDef, func() { + paramsDef.Status.Phase = parametersv1alpha1.PDAvailablePhase + })).Should(Succeed()) + Expect(testapps.GetAndChangeObj(&testCtx, client.ObjectKey{Name: compDefName}, func(compDef *appsv1.ComponentDefinition) { + compDef.Spec.Configs = []appsv1.ComponentFileTemplate{ + { + Name: "mysql-config", + Template: template.Name, + Namespace: template.Namespace, + VolumeName: "mysql-config", + ExternalManaged: pointer.Bool(true), + }, + } + })()).Should(Succeed()) + } + + It("fails fast in Action when the component does not support parameters", func() { + By("init operations resources with a componentDefinition without configs") + reqCtx := intctrlutil.RequestCtx{Ctx: ctx} + opsRes, _, _ := initOperationsResources(compDefName, clusterName) + createComponentObject() + + By("create a reconfigure opsRequest without the ComponentParameter object") + opsRes.OpsRequest = testops.CreateOpsRequest(ctx, testCtx, newReconfigureOps("unsupported-reconfigure-"+randomStr)) + Expect(opsutil.UpdateClusterOpsAnnotations(ctx, k8sClient, opsRes.Cluster, nil)).Should(Succeed()) + opsRes.OpsRequest.Status.Phase = opsv1alpha1.OpsPendingPhase + _, err := GetOpsManager().Do(reqCtx, k8sClient, opsRes) + Expect(err).ShouldNot(HaveOccurred()) + Eventually(testops.GetOpsRequestPhase(&testCtx, client.ObjectKeyFromObject(opsRes.OpsRequest))).Should(Equal(opsv1alpha1.OpsCreatingPhase)) + + By("expect the opsRequest to fail fast instead of retrying forever") + _, err = GetOpsManager().Do(reqCtx, k8sClient, opsRes) + Expect(err).ShouldNot(HaveOccurred()) + Eventually(testapps.CheckObj(&testCtx, client.ObjectKeyFromObject(opsRes.OpsRequest), func(g Gomega, fetched *opsv1alpha1.OpsRequest) { + g.Expect(fetched.Status.Phase).Should(Equal(opsv1alpha1.OpsFailedPhase)) + condition := meta.FindStatusCondition(fetched.Status.Conditions, opsv1alpha1.ConditionTypeFailed) + g.Expect(condition).ShouldNot(BeNil()) + g.Expect(condition.Message).Should(ContainSubstring("does not support reconfigure")) + })).Should(Succeed()) + }) + + It("fails fast in ReconcileAction when the component does not support parameters", func() { + By("init operations resources with a componentDefinition without configs") + reqCtx := intctrlutil.RequestCtx{Ctx: ctx} + opsRes, _, _ := initOperationsResources(compDefName, clusterName) + createComponentObject() + + By("create a running reconfigure opsRequest without the ComponentParameter object") + opsRes.OpsRequest = testops.CreateOpsRequest(ctx, testCtx, newReconfigureOps("unsupported-reconcile-"+randomStr)) + Expect(testapps.ChangeObjStatus(&testCtx, opsRes.OpsRequest, func() { + opsRes.OpsRequest.Status.Phase = opsv1alpha1.OpsRunningPhase + })).Should(Succeed()) + + By("expect the opsRequest to fail fast instead of retrying forever") + _, err := GetOpsManager().Reconcile(reqCtx, k8sClient, opsRes) + Expect(err).ShouldNot(HaveOccurred()) + Eventually(testapps.CheckObj(&testCtx, client.ObjectKeyFromObject(opsRes.OpsRequest), func(g Gomega, fetched *opsv1alpha1.OpsRequest) { + g.Expect(fetched.Status.Phase).Should(Equal(opsv1alpha1.OpsFailedPhase)) + condition := meta.FindStatusCondition(fetched.Status.Conditions, opsv1alpha1.ConditionTypeFailed) + g.Expect(condition).ShouldNot(BeNil()) + g.Expect(condition.Message).Should(ContainSubstring("does not support reconfigure")) + })).Should(Succeed()) + }) + + It("keeps waiting when the component supports parameters but the ComponentParameter is not created yet", func() { + By("init operations resources with a componentDefinition with valid parameter definitions") + reqCtx := intctrlutil.RequestCtx{Ctx: ctx} + opsRes, _, _ := initOperationsResources(compDefName, clusterName) + createComponentObject() + prepareParameterSupport() + + By("create a reconfigure opsRequest without the ComponentParameter object") + opsRes.OpsRequest = testops.CreateOpsRequest(ctx, testCtx, newReconfigureOps("waiting-reconfigure-"+randomStr)) + Expect(opsutil.UpdateClusterOpsAnnotations(ctx, k8sClient, opsRes.Cluster, nil)).Should(Succeed()) + opsRes.OpsRequest.Status.Phase = opsv1alpha1.OpsPendingPhase + _, err := GetOpsManager().Do(reqCtx, k8sClient, opsRes) + Expect(err).ShouldNot(HaveOccurred()) + Eventually(testops.GetOpsRequestPhase(&testCtx, client.ObjectKeyFromObject(opsRes.OpsRequest))).Should(Equal(opsv1alpha1.OpsCreatingPhase)) + + By("expect the Action path to keep retrying on the transient NotFound error") + _, err = GetOpsManager().Do(reqCtx, k8sClient, opsRes) + Expect(err).Should(HaveOccurred()) + Expect(apierrors.IsNotFound(err)).Should(BeTrue()) + Consistently(testops.GetOpsRequestPhase(&testCtx, client.ObjectKeyFromObject(opsRes.OpsRequest))).Should(Equal(opsv1alpha1.OpsCreatingPhase)) + + By("expect the Reconcile path to keep retrying on the transient NotFound error") + Expect(testapps.ChangeObjStatus(&testCtx, opsRes.OpsRequest, func() { + opsRes.OpsRequest.Status.Phase = opsv1alpha1.OpsRunningPhase + })).Should(Succeed()) + _, err = GetOpsManager().Reconcile(reqCtx, k8sClient, opsRes) + Expect(err).Should(HaveOccurred()) + Expect(apierrors.IsNotFound(err)).Should(BeTrue()) + Consistently(testops.GetOpsRequestPhase(&testCtx, client.ObjectKeyFromObject(opsRes.OpsRequest))).Should(Equal(opsv1alpha1.OpsRunningPhase)) + }) }) })