Skip to content

Commit

Permalink
chore: remove cd ref from backup policy template (#7940)
Browse files Browse the repository at this point in the history
  • Loading branch information
@leon-inf
leon-inf authored Aug 8, 2024
1 parent 513f2ab commit 1bccc47
Show file tree
Hide file tree
Showing 15 changed files with 30 additions and 439 deletions.
53 changes: 1 addition & 52 deletions apis/apps/v1alpha1/backuppolicytemplate_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,6 @@ import (

// BackupPolicyTemplateSpec contains the settings in a BackupPolicyTemplate.
type BackupPolicyTemplateSpec struct {
// Specifies the name of a ClusterDefinition.
// This is an immutable attribute that cannot be changed after creation.
// And this field is deprecated since v0.9, consider using the ComponentDef instead.
//
// +kubebuilder:validation:Pattern:=`^[a-z0-9]([a-z0-9\.\-]*[a-z0-9])?$`
// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="clusterDefinitionRef is immutable"
// +kubebuilder:deprecatedversion:warning="This field has been deprecated since 0.9.0, consider using the ComponentDef instead"
ClusterDefRef string `json:"clusterDefinitionRef,omitempty"`

// Represents an array of BackupPolicy templates, with each template corresponding to a specified ComponentDefinition
// or to a group of ComponentDefinitions that are different versions of definitions of the same component.
//
Expand All @@ -56,18 +47,6 @@ type BackupPolicyTemplateSpec struct {
// BackupPolicy is the template corresponding to a specified ComponentDefinition
// or to a group of ComponentDefinitions that are different versions of definitions of the same component.
type BackupPolicy struct {
// Specifies the name of ClusterComponentDefinition defined in the ClusterDefinition.
// Must comply with the IANA Service Naming rule.
//
// Deprecated since v0.9, should use `componentDefs` instead.
// This field is maintained for backward compatibility and its use is discouraged.
// Existing usage should be updated to the current preferred approach to avoid compatibility issues in future releases.
//
// +kubebuilder:validation:MaxLength=22
// +kubebuilder:validation:Pattern:=`^[a-z]([a-z0-9\-]*[a-z0-9])?$`
// +optional
ComponentDefRef string `json:"componentDefRef,omitempty"`

// Specifies a list of names of ComponentDefinitions that the specified ClusterDefinition references.
// They should be different versions of definitions of the same component,
// thus allowing them to share a single BackupPolicy.
Expand Down Expand Up @@ -220,9 +199,6 @@ type TargetInstance struct {
// This account must match one listed in `componentDefinition.spec.systemAccounts[*].name`.
// The corresponding secret created by this account is used to connect to the database.
//
// If `backupPolicy.componentDefRef` (a legacy and deprecated API) is set, the secret defined in
// `clusterDefinition.spec.ConnectionCredential` is used instead.
//
// +optional
Account string `json:"account,omitempty"`

Expand All @@ -235,37 +211,10 @@ type TargetInstance struct {
//
// +optional
Strategy dpv1alpha1.PodSelectionStrategy `json:"strategy,omitempty"`

// Specifies the keys of the connection credential secret defined in `clusterDefinition.spec.ConnectionCredential`.
// It will be ignored when the `account` is set.
//
// +optional
ConnectionCredentialKey ConnectionCredentialKey `json:"connectionCredentialKey,omitempty"`
}

type ConnectionCredentialKey struct {
// Represents the key of the password in the connection credential secret.
// If not specified, the default key "password" is used.
//
// +optional
PasswordKey *string `json:"passwordKey,omitempty"`

// Represents the key of the username in the connection credential secret.
// If not specified, the default key "username" is used.
//
// +optional
UsernameKey *string `json:"usernameKey,omitempty"`

// Defines the key of the host in the connection credential secret.
HostKey *string `json:"hostKey,omitempty"`

// Indicates map key of the port in the connection credential secret.
PortKey *string `json:"portKey,omitempty"`
}

// BackupPolicyTemplateStatus defines the observed state of BackupPolicyTemplate.
type BackupPolicyTemplateStatus struct {
}
type BackupPolicyTemplateStatus struct{}

// BackupPolicyTemplate should be provided by addon developers and is linked to a ClusterDefinition
// and its associated ComponentDefinitions.
Expand Down
40 changes: 2 additions & 38 deletions apis/apps/v1alpha1/zz_generated.deepcopy.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

78 changes: 0 additions & 78 deletions config/crd/bases/apps.kubeblocks.io_backuppolicytemplates.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -355,10 +355,6 @@ spec:
If `backupPolicy.componentDefs` is set, this field is required to specify the system account name.
This account must match one listed in `componentDefinition.spec.systemAccounts[*].name`.
The corresponding secret created by this account is used to connect to the database.
If `backupPolicy.componentDefRef` (a legacy and deprecated API) is set, the secret defined in
`clusterDefinition.spec.ConnectionCredential` is used instead.
type: string
connectionCredential:
description: Specifies the connection credential to
Expand Down Expand Up @@ -392,30 +388,6 @@ spec:
required:
- secretName
type: object
connectionCredentialKey:
description: |-
Specifies the keys of the connection credential secret defined in `clusterDefinition.spec.ConnectionCredential`.
It will be ignored when the `account` is set.
properties:
hostKey:
description: Defines the key of the host in the
connection credential secret.
type: string
passwordKey:
description: |-
Represents the key of the password in the connection credential secret.
If not specified, the default key "password" is used.
type: string
portKey:
description: Indicates map key of the port in
the connection credential secret.
type: string
usernameKey:
description: |-
Represents the key of the username in the connection credential secret.
If not specified, the default key "username" is used.
type: string
type: object
fallbackRole:
description: |-
Specifies the fallback role to select one replica for backup, this only takes effect when the
Expand Down Expand Up @@ -920,18 +892,6 @@ spec:
- name
type: object
type: array
componentDefRef:
description: |-
Specifies the name of ClusterComponentDefinition defined in the ClusterDefinition.
Must comply with the IANA Service Naming rule.
Deprecated since v0.9, should use `componentDefs` instead.
This field is maintained for backward compatibility and its use is discouraged.
Existing usage should be updated to the current preferred approach to avoid compatibility issues in future releases.
maxLength: 22
pattern: ^[a-z]([a-z0-9\-]*[a-z0-9])?$
type: string
componentDefs:
description: |-
Specifies a list of names of ComponentDefinitions that the specified ClusterDefinition references.
Expand Down Expand Up @@ -986,35 +946,7 @@ spec:
If `backupPolicy.componentDefs` is set, this field is required to specify the system account name.
This account must match one listed in `componentDefinition.spec.systemAccounts[*].name`.
The corresponding secret created by this account is used to connect to the database.
If `backupPolicy.componentDefRef` (a legacy and deprecated API) is set, the secret defined in
`clusterDefinition.spec.ConnectionCredential` is used instead.
type: string
connectionCredentialKey:
description: |-
Specifies the keys of the connection credential secret defined in `clusterDefinition.spec.ConnectionCredential`.
It will be ignored when the `account` is set.
properties:
hostKey:
description: Defines the key of the host in the connection
credential secret.
type: string
passwordKey:
description: |-
Represents the key of the password in the connection credential secret.
If not specified, the default key "password" is used.
type: string
portKey:
description: Indicates map key of the port in the connection
credential secret.
type: string
usernameKey:
description: |-
Represents the key of the username in the connection credential secret.
If not specified, the default key "username" is used.
type: string
type: object
fallbackRole:
description: |-
Specifies the fallback role to select one replica for backup, this only takes effect when the
Expand Down Expand Up @@ -1055,16 +987,6 @@ spec:
type: object
minItems: 1
type: array
clusterDefinitionRef:
description: |-
Specifies the name of a ClusterDefinition.
This is an immutable attribute that cannot be changed after creation.
And this field is deprecated since v0.9, consider using the ComponentDef instead.
pattern: ^[a-z0-9]([a-z0-9\.\-]*[a-z0-9])?$
type: string
x-kubernetes-validations:
- message: clusterDefinitionRef is immutable
rule: self == oldSelf
identifier:
description: |-
Specifies a unique identifier for the BackupPolicyTemplate.
Expand Down
6 changes: 0 additions & 6 deletions controllers/apps/backuppolicytemplate_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import (
"sigs.k8s.io/controller-runtime/pkg/reconcile"

appsv1alpha1 "github.com/apecloud/kubeblocks/apis/apps/v1alpha1"
"github.com/apecloud/kubeblocks/pkg/constant"
intctrlutil "github.com/apecloud/kubeblocks/pkg/controllerutil"
)

Expand All @@ -54,11 +53,6 @@ func (r *BackupPolicyTemplateReconciler) Reconcile(ctx context.Context, req reco
return intctrlutil.CheckedRequeueWithError(err, reqCtx.Log, "")
}

// infer clusterDefRef from spec.clusterDefRef
if backupPolicyTemplate.Spec.ClusterDefRef != "" {
backupPolicyTemplate.Labels[constant.ClusterDefLabelKey] = backupPolicyTemplate.Spec.ClusterDefRef
}

for _, backupPolicy := range backupPolicyTemplate.Spec.BackupPolicies {
for _, compDef := range backupPolicy.ComponentDefs {
backupPolicyTemplate.Labels[compDef] = compDef
Expand Down
8 changes: 1 addition & 7 deletions controllers/apps/backuppolicytemplate_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,13 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/apecloud/kubeblocks/apis/apps/v1alpha1"
"github.com/apecloud/kubeblocks/pkg/constant"
intctrlutil "github.com/apecloud/kubeblocks/pkg/generics"
testapps "github.com/apecloud/kubeblocks/pkg/testutil/apps"
)

var _ = Describe("", func() {
var (
BackupPolicyTemplateName = "test-bpt"
ClusterDefName = "test-cd"
BackupPolicyName = "test-bp"
BackupMethod = "test-bm"
ActionSetName = "test-as"
VsBackupMethodName = "test-vs-bm"
Expand Down Expand Up @@ -63,9 +60,7 @@ var _ = Describe("", func() {
compDef1 := "compDef1"
compDef2 := "compDef2"
bpt := testapps.NewBackupPolicyTemplateFactory(BackupPolicyTemplateName).
SetClusterDefRef(ClusterDefName).
AddBackupPolicy(BackupPolicyName).
SetComponentDef(compDef1, compDef2).
AddBackupPolicy(compDef1, compDef2).
AddBackupMethod(BackupMethod, false, ActionSetName).
SetBackupMethodVolumeMounts("data", "/data").
AddBackupMethod(VsBackupMethodName, true, "").
Expand All @@ -75,7 +70,6 @@ var _ = Describe("", func() {
Create(&testCtx).GetObject()
key := client.ObjectKeyFromObject(bpt)
Eventually(testapps.CheckObj(&testCtx, key, func(g Gomega, pobj *v1alpha1.BackupPolicyTemplate) {
g.Expect(pobj.GetLabels()[constant.ClusterDefLabelKey]).To(Equal(bpt.Spec.ClusterDefRef))
g.Expect(pobj.GetLabels()[compDef1]).To(Equal(compDef1))
g.Expect(pobj.GetLabels()[compDef2]).To(Equal(compDef2))
})).Should(Succeed())
Expand Down
1 change: 0 additions & 1 deletion controllers/apps/cluster_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1413,7 +1413,6 @@ func createBackupPolicyTpl(compDef string) {
ttl := "7d"
bpt = bpt.AddBackupPolicy(compDef).
AddBackupMethod(backupMethodName, false, actionSetName).
SetComponentDef(compDef).
SetBackupMethodVolumeMounts("data", "/data").
AddBackupMethod(vsBackupMethodName, true, "").
SetBackupMethodVolumes([]string{"data"}).
Expand Down
4 changes: 2 additions & 2 deletions controllers/apps/component_hscale_volume_populator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -445,8 +445,8 @@ func getBackupPolicyFromTemplate(reqCtx intctrlutil.RequestCtx,
if err := cli.List(reqCtx.Ctx, backupPolicyList,
client.InNamespace(cluster.Namespace),
client.MatchingLabels{
constant.AppInstanceLabelKey: cluster.Name,
constant.KBAppComponentDefRefLabelKey: componentDef,
constant.AppInstanceLabelKey: cluster.Name,
constant.ComponentDefinitionLabelKey: componentDef,
}); err != nil {
return nil, err
}
Expand Down
6 changes: 0 additions & 6 deletions controllers/apps/componentdefinition_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,6 @@ func (r *ComponentDefinitionReconciler) validate(cli client.Client, rctx intctrl
r.validateSystemAccounts,
r.validateReplicaRoles,
r.validateLifecycleActions,
r.validateComponentDefRef,
} {
if err := validator(cli, rctx, cmpd); err != nil {
return err
Expand Down Expand Up @@ -398,11 +397,6 @@ func (r *ComponentDefinitionReconciler) validateLifecycleActionBuiltInHandlers(l
return nil
}

func (r *ComponentDefinitionReconciler) validateComponentDefRef(cli client.Client, reqCtx intctrlutil.RequestCtx,
cmpd *appsv1alpha1.ComponentDefinition) error {
return nil
}

func (r *ComponentDefinitionReconciler) immutableCheck(cmpd *appsv1alpha1.ComponentDefinition) error {
if r.skipImmutableCheck(cmpd) {
return nil
Expand Down
Loading

0 comments on commit 1bccc47

Please sign in to comment.