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)) + }) }) })