Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions pkg/operations/reconfigure.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fatal classification violates reconcile semantics. ResolveCmpdParametersDefs returning no match describes the dependencies visible in this reconciliation; it is not a stable API fact that the component will never support parameters. The component-driven parameter controller explicitly watches ParametersDefinition and can create the missing ComponentParameter when a matching PD is created or becomes Available later. With this branch, a Reconfigure submitted before that event becomes terminally Failed before the system gets a chance to converge. Without an authoritative API-level terminal capability such as "parameters unsupported", one reconcile pass cannot infer "never" from currently absent dependencies.

"component %s does not support reconfigure: ComponentParameter not found", compName)
}
133 changes: 133 additions & 0 deletions pkg/operations/reconfigure_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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))
})
})
})
Loading