Skip to content

Commit

Permalink
chore: support resolving vars from multiple components consistently
Browse files Browse the repository at this point in the history
  • Loading branch information
@leon-inf
leon-inf committed Jan 16, 2025
1 parent 5be65e3 commit 8edcabe
Show file tree
Hide file tree
Showing 13 changed files with 461 additions and 21 deletions.
6 changes: 6 additions & 0 deletions apis/apps/v1/componentdefinition_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -877,6 +877,12 @@ type ClusterObjectReference struct {

// MultipleClusterObjectOption defines the options for handling multiple cluster objects matched.
type MultipleClusterObjectOption struct {
// RequireAllComponentObjects controls whether all component objects must exist before resolving.
// If set to true, resolving will only proceed if all component objects are present.
//
// +optional
RequireAllComponentObjects *bool `json:"requireAllComponentObjects,omitempty"`

// Define the strategy for handling multiple cluster objects.
//
// +kubebuilder:validation:Required
Expand Down
5 changes: 5 additions & 0 deletions apis/apps/v1/zz_generated.deepcopy.go
View file

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

30 changes: 30 additions & 0 deletions config/crd/bases/apps.kubeblocks.io_componentdefinitions.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16986,6 +16986,11 @@ spec:
components.
type: string
type: object
requireAllComponentObjects:
description: |-
RequireAllComponentObjects controls whether all component objects must exist before resolving.
If set to true, resolving will only proceed if all component objects are present.
type: boolean
strategy:
description: Define the strategy for handling multiple
cluster objects.
Expand Down Expand Up @@ -17133,6 +17138,11 @@ spec:
components.
type: string
type: object
requireAllComponentObjects:
description: |-
RequireAllComponentObjects controls whether all component objects must exist before resolving.
If set to true, resolving will only proceed if all component objects are present.
type: boolean
strategy:
description: Define the strategy for handling multiple
cluster objects.
Expand Down Expand Up @@ -17238,6 +17248,11 @@ spec:
components.
type: string
type: object
requireAllComponentObjects:
description: |-
RequireAllComponentObjects controls whether all component objects must exist before resolving.
If set to true, resolving will only proceed if all component objects are present.
type: boolean
strategy:
description: Define the strategy for handling multiple
cluster objects.
Expand Down Expand Up @@ -17341,6 +17356,11 @@ spec:
components.
type: string
type: object
requireAllComponentObjects:
description: |-
RequireAllComponentObjects controls whether all component objects must exist before resolving.
If set to true, resolving will only proceed if all component objects are present.
type: boolean
strategy:
description: Define the strategy for handling multiple
cluster objects.
Expand Down Expand Up @@ -17454,6 +17474,11 @@ spec:
components.
type: string
type: object
requireAllComponentObjects:
description: |-
RequireAllComponentObjects controls whether all component objects must exist before resolving.
If set to true, resolving will only proceed if all component objects are present.
type: boolean
strategy:
description: Define the strategy for handling multiple
cluster objects.
Expand Down Expand Up @@ -17554,6 +17579,11 @@ spec:
components.
type: string
type: object
requireAllComponentObjects:
description: |-
RequireAllComponentObjects controls whether all component objects must exist before resolving.
If set to true, resolving will only proceed if all component objects are present.
type: boolean
strategy:
description: Define the strategy for handling multiple
cluster objects.
Expand Down
30 changes: 30 additions & 0 deletions config/crd/bases/apps.kubeblocks.io_sidecardefinitions.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1683,6 +1683,11 @@ spec:
components.
type: string
type: object
requireAllComponentObjects:
description: |-
RequireAllComponentObjects controls whether all component objects must exist before resolving.
If set to true, resolving will only proceed if all component objects are present.
type: boolean
strategy:
description: Define the strategy for handling multiple
cluster objects.
Expand Down Expand Up @@ -1830,6 +1835,11 @@ spec:
components.
type: string
type: object
requireAllComponentObjects:
description: |-
RequireAllComponentObjects controls whether all component objects must exist before resolving.
If set to true, resolving will only proceed if all component objects are present.
type: boolean
strategy:
description: Define the strategy for handling multiple
cluster objects.
Expand Down Expand Up @@ -1935,6 +1945,11 @@ spec:
components.
type: string
type: object
requireAllComponentObjects:
description: |-
RequireAllComponentObjects controls whether all component objects must exist before resolving.
If set to true, resolving will only proceed if all component objects are present.
type: boolean
strategy:
description: Define the strategy for handling multiple
cluster objects.
Expand Down Expand Up @@ -2038,6 +2053,11 @@ spec:
components.
type: string
type: object
requireAllComponentObjects:
description: |-
RequireAllComponentObjects controls whether all component objects must exist before resolving.
If set to true, resolving will only proceed if all component objects are present.
type: boolean
strategy:
description: Define the strategy for handling multiple
cluster objects.
Expand Down Expand Up @@ -2151,6 +2171,11 @@ spec:
components.
type: string
type: object
requireAllComponentObjects:
description: |-
RequireAllComponentObjects controls whether all component objects must exist before resolving.
If set to true, resolving will only proceed if all component objects are present.
type: boolean
strategy:
description: Define the strategy for handling multiple
cluster objects.
Expand Down Expand Up @@ -2251,6 +2276,11 @@ spec:
components.
type: string
type: object
requireAllComponentObjects:
description: |-
RequireAllComponentObjects controls whether all component objects must exist before resolving.
If set to true, resolving will only proceed if all component objects are present.
type: boolean
strategy:
description: Define the strategy for handling multiple
cluster objects.
Expand Down
54 changes: 50 additions & 4 deletions controllers/apps/shardingdefinition_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,21 +190,67 @@ func (r *ShardingDefinitionReconciler) validateShardsLimit(ctx context.Context,

func (r *ShardingDefinitionReconciler) validateProvisionNUpdateStrategy(ctx context.Context, cli client.Client,
shardingDef *appsv1.ShardingDefinition) error {
var (
provision = shardingDef.Spec.ProvisionStrategy
update = shardingDef.Spec.UpdateStrategy
)

supported := func(strategy *appsv1.UpdateStrategy) bool {
if strategy == nil {
return true
}
return *strategy == appsv1.SerialStrategy || *strategy == appsv1.ParallelStrategy
}
if !supported(shardingDef.Spec.ProvisionStrategy) {
return fmt.Errorf("unsupported provision strategy: %s", *shardingDef.Spec.ProvisionStrategy)
if !supported(provision) {
return fmt.Errorf("unsupported provision strategy: %s", *provision)
}
if !supported(update) {
return fmt.Errorf("unsupported update strategy: %s", *update)
}
if !supported(shardingDef.Spec.UpdateStrategy) {
return fmt.Errorf("unsupported update strategy: %s", *shardingDef.Spec.UpdateStrategy)

if provision != nil && *provision == appsv1.SerialStrategy && r.requireParallelProvision() {
return fmt.Errorf("serial provision strategy is conflicted with vars that requires parallel provision when mutiple objects matched")
}
return nil
}

func (r *ShardingDefinitionReconciler) requireParallelProvision() bool {
requireAll := func(opt *appsv1.MultipleClusterObjectOption) bool {
return opt != nil && opt.RequireAllComponentObjects != nil && *opt.RequireAllComponentObjects
}
require := func(v appsv1.EnvVar) bool {
if v.ValueFrom != nil {
if v.ValueFrom.HostNetworkVarRef != nil {
return requireAll(v.ValueFrom.HostNetworkVarRef.MultipleClusterObjectOption)
}
if v.ValueFrom.ServiceVarRef != nil {
return requireAll(v.ValueFrom.ServiceVarRef.MultipleClusterObjectOption)
}
if v.ValueFrom.CredentialVarRef != nil {
return requireAll(v.ValueFrom.CredentialVarRef.MultipleClusterObjectOption)
}
if v.ValueFrom.TLSVarRef != nil {
return requireAll(v.ValueFrom.TLSVarRef.MultipleClusterObjectOption)
}
if v.ValueFrom.ServiceRefVarRef != nil {
return requireAll(v.ValueFrom.ServiceRefVarRef.MultipleClusterObjectOption)
}
if v.ValueFrom.ComponentVarRef != nil {
return requireAll(v.ValueFrom.ComponentVarRef.MultipleClusterObjectOption)
}
}
return false
}
for _, compDef := range r.compDefs {
for _, v := range compDef.Spec.Vars {
if require(v) {
return true
}
}
}
return false
}

func (r *ShardingDefinitionReconciler) validateLifecycleActions(ctx context.Context, cli client.Client,
shardingDef *appsv1.ShardingDefinition) error {
return nil
Expand Down
80 changes: 77 additions & 3 deletions controllers/apps/shardingdefinition_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import (
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"

"k8s.io/utils/pointer"
"k8s.io/utils/ptr"
"sigs.k8s.io/controller-runtime/pkg/client"

appsv1 "github.com/apecloud/kubeblocks/apis/apps/v1"
Expand Down Expand Up @@ -131,13 +131,87 @@ var _ = Describe("ShardingDefinition Controller", func() {

checkObjectStatus(shardingDefObj, appsv1.UnavailablePhase)
})

It("provision strategy & vars - serial", func() {
By("create a ShardingDefinition obj")
shardingDefObj := testapps.NewShardingDefinitionFactory(shardingDefName, compDefObj.GetName()).
SetProvisionStrategy(appsv1.SerialStrategy).
SetUpdateStrategy(appsv1.SerialStrategy).
Create(&testCtx).GetObject()

checkObjectStatus(shardingDefObj, appsv1.AvailablePhase)
})

It("provision strategy & vars - parallel & all", func() {
By("add a var in cmpd")
Expect(testapps.GetAndChangeObj(&testCtx, client.ObjectKeyFromObject(compDefObj), func(compDef *appsv1.ComponentDefinition) {
compDef.Spec.Vars = []appsv1.EnvVar{
{
Name: "test",
ValueFrom: &appsv1.VarSource{
ComponentVarRef: &appsv1.ComponentVarSelector{
ClusterObjectReference: appsv1.ClusterObjectReference{
MultipleClusterObjectOption: &appsv1.MultipleClusterObjectOption{
RequireAllComponentObjects: ptr.To(true),
Strategy: appsv1.MultipleClusterObjectStrategyCombined,
},
},
ComponentVars: appsv1.ComponentVars{
PodFQDNs: &appsv1.VarRequired,
},
},
},
},
}
})()).Should(Succeed())

By("create a ShardingDefinition obj")
shardingDefObj := testapps.NewShardingDefinitionFactory(shardingDefName, compDefObj.GetName()).
SetProvisionStrategy(appsv1.ParallelStrategy).
SetUpdateStrategy(appsv1.ParallelStrategy).
Create(&testCtx).GetObject()

checkObjectStatus(shardingDefObj, appsv1.AvailablePhase)
})

It("provision strategy & vars - serial & all", func() {
By("add a var in cmpd")
Expect(testapps.GetAndChangeObj(&testCtx, client.ObjectKeyFromObject(compDefObj), func(compDef *appsv1.ComponentDefinition) {
compDef.Spec.Vars = []appsv1.EnvVar{
{
Name: "test",
ValueFrom: &appsv1.VarSource{
ComponentVarRef: &appsv1.ComponentVarSelector{
ClusterObjectReference: appsv1.ClusterObjectReference{
MultipleClusterObjectOption: &appsv1.MultipleClusterObjectOption{
RequireAllComponentObjects: ptr.To(true),
Strategy: appsv1.MultipleClusterObjectStrategyCombined,
},
},
ComponentVars: appsv1.ComponentVars{
PodFQDNs: &appsv1.VarRequired,
},
},
},
},
}
})()).Should(Succeed())

By("create a ShardingDefinition obj")
shardingDefObj := testapps.NewShardingDefinitionFactory(shardingDefName, compDefObj.GetName()).
SetProvisionStrategy(appsv1.SerialStrategy).
SetUpdateStrategy(appsv1.SerialStrategy).
Create(&testCtx).GetObject()

checkObjectStatus(shardingDefObj, appsv1.UnavailablePhase)
})
})

Context("system accounts", func() {
It("ok", func() {
By("create a ShardingDefinition obj")
shardingDefObj := testapps.NewShardingDefinitionFactory(shardingDefName, compDefObj.GetName()).
AddSystemAccount(appsv1.ShardingSystemAccount{Name: adminAccount, Shared: pointer.Bool(true)}).
AddSystemAccount(appsv1.ShardingSystemAccount{Name: adminAccount, Shared: ptr.To(true)}).
Create(&testCtx).GetObject()

checkObjectStatus(shardingDefObj, appsv1.AvailablePhase)
Expand All @@ -147,7 +221,7 @@ var _ = Describe("ShardingDefinition Controller", func() {
By("create a ShardingDefinition obj")
shardingDefObj := testapps.NewShardingDefinitionFactory(shardingDefName, compDefObj.GetName()).
AddSystemAccount(appsv1.ShardingSystemAccount{Name: adminAccount}).
AddSystemAccount(appsv1.ShardingSystemAccount{Name: adminAccount, Shared: pointer.Bool(true)}).
AddSystemAccount(appsv1.ShardingSystemAccount{Name: adminAccount, Shared: ptr.To(true)}).
Create(&testCtx).GetObject()

checkObjectStatus(shardingDefObj, appsv1.UnavailablePhase)
Expand Down
Loading

0 comments on commit 8edcabe

Please sign in to comment.