Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: filtering v1 version events fails when no changes have been made to the ClusterDefinition. #8612

Merged
merged 8 commits into from
Dec 11, 2024
Merged
Show file tree
Hide file tree
Changes from 6 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
7 changes: 3 additions & 4 deletions apis/apps/v1alpha1/cluster_conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,6 @@ func (r *Cluster) changesToCluster(cluster *appsv1.Cluster) {
func (r *Cluster) changesFromCluster(cluster *appsv1.Cluster) {
// changed:
// spec
// clusterDefRef -> clusterDef
// components
// - volumeClaimTemplates
// spec:
Expand All @@ -142,9 +141,6 @@ func (r *Cluster) changesFromCluster(cluster *appsv1.Cluster) {
// status
// components
// - message: ComponentMessageMap -> map[string]string
if len(cluster.Spec.ClusterDef) > 0 {
r.Spec.ClusterDefRef = cluster.Spec.ClusterDef
}
// appsv1.TerminationPolicyType is a subset of appsv1alpha1.TerminationPolicyType, it can be converted directly.
}

Expand All @@ -154,6 +150,7 @@ type clusterConverter struct {
}

type clusterSpecConverter struct {
ClusterDefRef string `json:"clusterDefRef,omitempty"`
ClusterVersionRef string `json:"clusterVersionRef,omitempty"`
TerminationPolicy TerminationPolicyType `json:"terminationPolicy"`
Affinity *Affinity `json:"affinity,omitempty"`
Expand Down Expand Up @@ -193,6 +190,7 @@ type clusterCompStatusConverter struct {

func (c *clusterConverter) fromCluster(cluster *Cluster) {
c.Spec.ClusterVersionRef = cluster.Spec.ClusterVersionRef
c.Spec.ClusterDefRef = cluster.Spec.ClusterDefRef
c.Spec.TerminationPolicy = cluster.Spec.TerminationPolicy
c.Spec.Affinity = cluster.Spec.Affinity
c.Spec.Tolerations = cluster.Spec.Tolerations
Expand Down Expand Up @@ -244,6 +242,7 @@ func (c *clusterConverter) fromCluster(cluster *Cluster) {

func (c *clusterConverter) toCluster(cluster *Cluster) {
cluster.Spec.ClusterVersionRef = c.Spec.ClusterVersionRef
cluster.Spec.ClusterDefRef = c.Spec.ClusterDefRef
cluster.Spec.TerminationPolicy = c.Spec.TerminationPolicy
cluster.Spec.Affinity = c.Spec.Affinity
cluster.Spec.Tolerations = c.Spec.Tolerations
Expand Down
13 changes: 7 additions & 6 deletions apis/apps/v1alpha1/conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import (
)

const (
kbIncrementConverterAK = "kb-increment-converter"
KBIncrementConverterAK = "kb-increment-converter"
)

type incrementChange any
Expand Down Expand Up @@ -62,7 +62,7 @@ func incrementConvertTo(converter incrementConverter, target metav1.Object) erro
if annotations == nil {
annotations = make(map[string]string)
}
annotations[kbIncrementConverterAK] = string(bytes)
annotations[KBIncrementConverterAK] = string(bytes)
target.SetAnnotations(annotations)

return nil
Expand All @@ -75,15 +75,16 @@ func incrementConvertFrom(converter incrementConverter, source metav1.Object, ic

annotations := source.GetAnnotations()
if annotations != nil {
data, ok := annotations[kbIncrementConverterAK]
data, ok := annotations[KBIncrementConverterAK]
if ok {
// Convert from the incremental converter only if the annotation exists.
if err := json.Unmarshal([]byte(data), ic); err != nil {
return err
}
delete(annotations, kbIncrementConverterAK)
delete(annotations, KBIncrementConverterAK)
source.SetAnnotations(annotations)
return converter.incrementConvertFrom(source, ic)
}
}

return converter.incrementConvertFrom(source, ic)
return nil
}
72 changes: 51 additions & 21 deletions controllers/apps/transformer_cluster_normalization.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ along with this program. If not, see <http://www.gnu.org/licenses/>.
package apps

import (
"encoding/json"
"fmt"

"golang.org/x/exp/maps"
Expand All @@ -29,6 +30,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"

appsv1 "github.com/apecloud/kubeblocks/apis/apps/v1"
appsv1alpha1 "github.com/apecloud/kubeblocks/apis/apps/v1alpha1"
"github.com/apecloud/kubeblocks/pkg/constant"
"github.com/apecloud/kubeblocks/pkg/controller/component"
"github.com/apecloud/kubeblocks/pkg/controller/graph"
Expand Down Expand Up @@ -416,37 +418,65 @@ func (t *clusterNormalizationTransformer) writeBackCompNShardingSpecs(transCtx *
}

func (t *clusterNormalizationTransformer) checkNPatchCRDAPIVersionKey(transCtx *clusterTransformContext) error {
apiVersions := map[string][]string{}

from := func(name string, annotations map[string]string) {
key := annotations[constant.CRDAPIVersionAnnotationKey]
apiVersions[key] = append(apiVersions[key], name)
apiVersion := transCtx.Cluster.Annotations[constant.CRDAPIVersionAnnotationKey]
getClusterDefFromIncrementConverter := func() (string, error) {
leon-inf marked this conversation as resolved.
Show resolved Hide resolved
incrementConverterStr := transCtx.Cluster.Annotations[appsv1alpha1.KBIncrementConverterAK]
if len(incrementConverterStr) == 0 {
return "", nil
}
var alpha1Cluster appsv1alpha1.Cluster
if err := json.Unmarshal([]byte(incrementConverterStr), &alpha1Cluster); err != nil {
return "", err
}
return alpha1Cluster.Spec.ClusterDefRef, nil
}

if transCtx.clusterDef != nil {
from(transCtx.clusterDef.Name, transCtx.clusterDef.Annotations)
} else {
for _, compDef := range transCtx.componentDefs {
from(compDef.Name, compDef.Annotations)
setCRDAPIVersion := func() error {
leon-inf marked this conversation as resolved.
Show resolved Hide resolved
apiVersions := map[string][]string{}

from := func(name string, annotations map[string]string) {
key := annotations[constant.CRDAPIVersionAnnotationKey]
apiVersions[key] = append(apiVersions[key], name)
}
for _, shardingDef := range transCtx.shardingDefs {
from(shardingDef.Name, shardingDef.Annotations)

clusterDefRef, err := getClusterDefFromIncrementConverter()
if err != nil {
return err
}
switch {
case len(clusterDefRef) != 0:
apiVersion = appsv1alpha1.GroupVersion.String()
case transCtx.clusterDef != nil:
from(transCtx.clusterDef.Name, transCtx.clusterDef.Annotations)
default:
for _, compDef := range transCtx.componentDefs {
from(compDef.Name, compDef.Annotations)
}
for _, shardingDef := range transCtx.shardingDefs {
from(shardingDef.Name, shardingDef.Annotations)
}
}
}

if len(apiVersions) > 1 {
return fmt.Errorf("multiple CRD API versions found: %v", apiVersions)
}
if len(apiVersions) > 1 {
return fmt.Errorf("multiple CRD API versions found: %v", apiVersions)
}

apiVersion := ""
if len(apiVersions) == 1 {
apiVersion = maps.Keys(apiVersions)[0]
if transCtx.Cluster.Annotations == nil {
transCtx.Cluster.Annotations = make(map[string]string)
if len(apiVersions) == 1 {
apiVersion = maps.Keys(apiVersions)[0]
if transCtx.Cluster.Annotations == nil {
leon-inf marked this conversation as resolved.
Show resolved Hide resolved
transCtx.Cluster.Annotations = make(map[string]string)
}
}
transCtx.Cluster.Annotations[constant.CRDAPIVersionAnnotationKey] = apiVersion
return nil
}

if len(apiVersion) == 0 {
// If the cluster does not specify a CRD version, default to setting it.
if err := setCRDAPIVersion(); err != nil {
return err
}
}
if controllerutil.IsAPIVersionSupported(apiVersion) {
return nil
}
Expand Down
7 changes: 3 additions & 4 deletions controllers/apps/transformer_component_init.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,15 @@ var _ graph.Transformer = &componentInitTransformer{}
func (t *componentInitTransformer) Transform(ctx graph.TransformContext, dag *graph.DAG) error {
transCtx, _ := ctx.(*componentTransformContext)

if !intctrlutil.ObjectAPIVersionSupported(transCtx.Component) {
return graph.ErrPrematureStop
}

// init dag
rootVertex := &model.ObjectVertex{Obj: transCtx.Component, OriObj: transCtx.ComponentOrig, Action: model.ActionStatusPtr()}
dag.AddVertex(rootVertex)

// init placement
transCtx.Context = intoContext(transCtx.Context, placement(transCtx.Component))

if !intctrlutil.ObjectAPIVersionSupported(transCtx.Component) {
return graph.ErrPrematureStop
}
return nil
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# permissions for end users to edit sidecardefinitions.
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
name: {{ include "kubeblocks.fullname" . }}-sidecardefinition-editor-role
labels:
{{- include "kubeblocks.labels" . | nindent 4 }}
rules:
- apiGroups:
- apps.kubeblocks.io
resources:
- sidecardefinitions
verbs:
- create
- delete
- get
- list
- patch
- update
- watch
- apiGroups:
- apps.kubeblocks.io
resources:
- sidecardefinitions/status
verbs:
- get
Loading