From 0564dbac500b05f748911efa60f083dfca1c42de Mon Sep 17 00:00:00 2001 From: Kacper Rzetelski Date: Thu, 29 Aug 2024 16:55:34 +0200 Subject: [PATCH 1/3] Aggregate NodeConfig status conditions and add server-side printers --- pkg/api/scylla/v1alpha1/types.go | 9 + pkg/api/scylla/v1alpha1/types_monitoring.go | 6 - pkg/api/scylla/v1alpha1/types_nodeconfig.go | 47 ++- pkg/controller/nodeconfig/conditions.go | 20 ++ pkg/controller/nodeconfig/status.go | 50 +--- pkg/controller/nodeconfig/sync.go | 271 ++++++++++++++++-- .../nodeconfig/sync_clusterrolebindings.go | 18 +- .../nodeconfig/sync_clusterroles.go | 15 +- pkg/controller/nodeconfig/sync_daemonsets.go | 40 ++- pkg/controller/nodeconfig/sync_namespaces.go | 15 +- .../nodeconfig/sync_rolebindings.go | 16 +- pkg/controller/nodeconfig/sync_roles.go | 16 +- .../nodeconfig/sync_serviceaccounts.go | 18 +- pkg/controller/nodesetup/status.go | 32 +-- pkg/controller/nodesetup/sync.go | 80 +++++- pkg/controllerhelpers/scylla.go | 106 ------- pkg/controllerhelpers/scylla_test.go | 153 ---------- pkg/internalapi/conditions.go | 11 +- 18 files changed, 514 insertions(+), 409 deletions(-) create mode 100644 pkg/api/scylla/v1alpha1/types.go create mode 100644 pkg/controller/nodeconfig/conditions.go diff --git a/pkg/api/scylla/v1alpha1/types.go b/pkg/api/scylla/v1alpha1/types.go new file mode 100644 index 00000000000..ca43dce4398 --- /dev/null +++ b/pkg/api/scylla/v1alpha1/types.go @@ -0,0 +1,9 @@ +// Copyright (C) 2024 ScyllaDB + +package v1alpha1 + +const ( + AvailableCondition = "Available" + ProgressingCondition = "Progressing" + DegradedCondition = "Degraded" +) diff --git a/pkg/api/scylla/v1alpha1/types_monitoring.go b/pkg/api/scylla/v1alpha1/types_monitoring.go index 3529a769c48..4e78877add4 100644 --- a/pkg/api/scylla/v1alpha1/types_monitoring.go +++ b/pkg/api/scylla/v1alpha1/types_monitoring.go @@ -5,12 +5,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -const ( - AvailableCondition = "Available" - ProgressingCondition = "Progressing" - DegradedCondition = "Degraded" -) - // GrafanaExposeOptions holds options related to exposing Grafana app. type GrafanaExposeOptions struct { // webInterface specifies expose options for the user web interface. diff --git a/pkg/api/scylla/v1alpha1/types_nodeconfig.go b/pkg/api/scylla/v1alpha1/types_nodeconfig.go index 77e88efb501..86bd3c0faa5 100644 --- a/pkg/api/scylla/v1alpha1/types_nodeconfig.go +++ b/pkg/api/scylla/v1alpha1/types_nodeconfig.go @@ -17,6 +17,7 @@ limitations under the License. package v1alpha1 import ( + "github.com/scylladb/scylla-operator/pkg/helpers/slices" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -25,10 +26,13 @@ import ( type NodeConfigConditionType string const ( - // Reconciled indicates that the NodeConfig is fully deployed and available. + // NodeConfigReconciledConditionType indicates that the NodeConfig is fully deployed and available. + // Deprecated: NodeConfigReconciledConditionType is deprecated. Use standard workload conditions instead. NodeConfigReconciledConditionType NodeConfigConditionType = "Reconciled" ) +// TODO(rzetelskik): move from NodeConfigCondition to metav1.Condition in next API version. + type NodeConfigCondition struct { // type is the type of the NodeConfig condition. Type NodeConfigConditionType `json:"type"` @@ -53,19 +57,55 @@ type NodeConfigCondition struct { Message string `json:"message"` } +func (c *NodeConfigCondition) ToMetaV1Condition() metav1.Condition { + return metav1.Condition{ + Type: string(c.Type), + Status: metav1.ConditionStatus(c.Status), + ObservedGeneration: c.ObservedGeneration, + LastTransitionTime: c.LastTransitionTime, + Reason: c.Reason, + Message: c.Message, + } +} + +func NewNodeConfigCondition(c metav1.Condition) NodeConfigCondition { + return NodeConfigCondition{ + Type: NodeConfigConditionType(c.Type), + Status: corev1.ConditionStatus(c.Status), + ObservedGeneration: c.ObservedGeneration, + LastTransitionTime: c.LastTransitionTime, + Reason: c.Reason, + Message: c.Message, + } +} + +type NodeConfigConditions []NodeConfigCondition + type NodeConfigNodeStatus struct { Name string `json:"name"` TunedNode bool `json:"tunedNode"` TunedContainers []string `json:"tunedContainers"` } +func (c NodeConfigConditions) ToMetaV1Conditions() []metav1.Condition { + return slices.ConvertSlice(c, func(from NodeConfigCondition) metav1.Condition { + return from.ToMetaV1Condition() + }) +} + +func NewNodeConfigConditions(cs []metav1.Condition) NodeConfigConditions { + return slices.ConvertSlice(cs, func(from metav1.Condition) NodeConfigCondition { + return NewNodeConfigCondition(from) + }) +} + type NodeConfigStatus struct { // observedGeneration indicates the most recent generation observed by the controller. ObservedGeneration int64 `json:"observedGeneration"` // conditions represents the latest available observations of current state. // +optional - Conditions []NodeConfigCondition `json:"conditions"` + Conditions NodeConfigConditions `json:"conditions"` // nodeStatuses hold the status for each tuned node. NodeStatuses []NodeConfigNodeStatus `json:"nodeStatuses"` @@ -206,6 +246,9 @@ type NodeConfigSpec struct { // +genclient // +genclient:nonNamespaced // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object +// +kubebuilder:printcolumn:name="AVAILABLE",type=string,JSONPath=".status.conditions[?(@.type=='Available')].status" +// +kubebuilder:printcolumn:name="PROGRESSING",type=string,JSONPath=".status.conditions[?(@.type=='Progressing')].status" +// +kubebuilder:printcolumn:name="DEGRADED",type=string,JSONPath=".status.conditions[?(@.type=='Degraded')].status" // +kubebuilder:printcolumn:name="AGE",type="date",JSONPath=".metadata.creationTimestamp" type NodeConfig struct { diff --git a/pkg/controller/nodeconfig/conditions.go b/pkg/controller/nodeconfig/conditions.go new file mode 100644 index 00000000000..a881501bde8 --- /dev/null +++ b/pkg/controller/nodeconfig/conditions.go @@ -0,0 +1,20 @@ +// Copyright (C) 2024 ScyllaDB + +package nodeconfig + +const ( + namespaceControllerProgressingCondition = "NamespaceControllerProgressing" + namespaceControllerDegradedCondition = "NamespaceControllerDegraded" + clusterRoleControllerProgressingCondition = "ClusterRoleControllerProgressing" + clusterRoleControllerDegradedCondition = "ClusterRoleControllerDegraded" + roleControllerProgressingCondition = "RoleControllerProgressing" + roleControllerDegradedCondition = "RoleControllerDegraded" + serviceAccountControllerProgressingCondition = "ServiceAccountControllerProgressing" + serviceAccountControllerDegradedCondition = "ServiceAccountControllerDegraded" + clusterRoleBindingControllerProgressingCondition = "ClusterRoleBindingControllerProgressing" + clusterRoleBindingControllerDegradedCondition = "ClusterRoleBindingControllerDegraded" + roleBindingControllerProgressingCondition = "RoleBindingControllerProgressing" + roleBindingControllerDegradedCondition = "RoleBindingControllerDegraded" + daemonSetControllerProgressingCondition = "DaemonSetControllerProgressing" + daemonSetControllerDegradedCondition = "DaemonSetControllerDegraded" +) diff --git a/pkg/controller/nodeconfig/status.go b/pkg/controller/nodeconfig/status.go index 0d4d460eaa9..fa5499176ec 100644 --- a/pkg/controller/nodeconfig/status.go +++ b/pkg/controller/nodeconfig/status.go @@ -4,54 +4,18 @@ package nodeconfig import ( "context" - "fmt" scyllav1alpha1 "github.com/scylladb/scylla-operator/pkg/api/scylla/v1alpha1" - "github.com/scylladb/scylla-operator/pkg/controllerhelpers" - "github.com/scylladb/scylla-operator/pkg/naming" - appsv1 "k8s.io/api/apps/v1" - corev1 "k8s.io/api/core/v1" apiequality "k8s.io/apimachinery/pkg/api/equality" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/klog/v2" ) -func (ncc *Controller) calculateStatus(nc *scyllav1alpha1.NodeConfig, daemonSets map[string]*appsv1.DaemonSet, scyllaUtilsImage string) (*scyllav1alpha1.NodeConfigStatus, error) { +func (ncc *Controller) calculateStatus(nc *scyllav1alpha1.NodeConfig) *scyllav1alpha1.NodeConfigStatus { status := nc.Status.DeepCopy() status.ObservedGeneration = nc.Generation - // TODO: We need to restructure the pattern so status update already know the desired object and we don't - // construct it twice. - requiredDaemonSet := makeNodeSetupDaemonSet(nc, ncc.operatorImage, scyllaUtilsImage) - ds, found := daemonSets[requiredDaemonSet.Name] - if !found { - klog.V(4).InfoS("Existing DaemonSet not found", "DaemonSet", klog.KObj(ds)) - return status, nil - } - - reconciled, err := controllerhelpers.IsDaemonSetRolledOut(ds) - if err != nil { - return nil, fmt.Errorf("can't determine is a daemonset %q is reconiled: %w", naming.ObjRef(ds), err) - } - - cond := scyllav1alpha1.NodeConfigCondition{ - Type: scyllav1alpha1.NodeConfigReconciledConditionType, - ObservedGeneration: nc.Generation, - } - - if reconciled { - cond.Status = corev1.ConditionTrue - cond.Reason = "FullyReconciledAndUp" - cond.Message = "All operands are reconciled and available." - } else { - cond.Status = corev1.ConditionFalse - cond.Reason = "DaemonSetNotRolledOut" - cond.Message = "DaemonSet isn't reconciled and fully rolled out yet." - } - - controllerhelpers.SetNodeConfigStatusCondition(&status.Conditions, cond) - - return status, nil + return status } func (ncc *Controller) updateStatus(ctx context.Context, currentNodeConfig *scyllav1alpha1.NodeConfig, status *scyllav1alpha1.NodeConfigStatus) error { @@ -59,17 +23,17 @@ func (ncc *Controller) updateStatus(ctx context.Context, currentNodeConfig *scyl return nil } - soc := currentNodeConfig.DeepCopy() - soc.Status = *status + nc := currentNodeConfig.DeepCopy() + nc.Status = *status - klog.V(2).InfoS("Updating status", "NodeConfig", klog.KObj(soc)) + klog.V(2).InfoS("Updating status", "NodeConfig", klog.KObj(nc)) - _, err := ncc.scyllaClient.NodeConfigs().UpdateStatus(ctx, soc, metav1.UpdateOptions{}) + _, err := ncc.scyllaClient.NodeConfigs().UpdateStatus(ctx, nc, metav1.UpdateOptions{}) if err != nil { return err } - klog.V(2).InfoS("Status updated", "NodeConfig", klog.KObj(soc)) + klog.V(2).InfoS("Status updated", "NodeConfig", klog.KObj(nc)) return nil } diff --git a/pkg/controller/nodeconfig/sync.go b/pkg/controller/nodeconfig/sync.go index 777d0cedb27..eaf8f1a51c8 100644 --- a/pkg/controller/nodeconfig/sync.go +++ b/pkg/controller/nodeconfig/sync.go @@ -9,11 +9,15 @@ import ( scyllav1alpha1 "github.com/scylladb/scylla-operator/pkg/api/scylla/v1alpha1" "github.com/scylladb/scylla-operator/pkg/controllerhelpers" + "github.com/scylladb/scylla-operator/pkg/helpers" + "github.com/scylladb/scylla-operator/pkg/internalapi" "github.com/scylladb/scylla-operator/pkg/naming" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" + apimeta "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/client-go/tools/cache" @@ -103,54 +107,219 @@ func (ncc *Controller) sync(ctx context.Context, key string) error { return objectErr } - status, err := ncc.calculateStatus(nc, daemonSets, ncc.operatorImage) - if err != nil { - return fmt.Errorf("can't calculate status: %w", err) - } + status := ncc.calculateStatus(nc) if nc.DeletionTimestamp != nil { return ncc.updateStatus(ctx, nc, status) } + statusConditions := status.Conditions.ToMetaV1Conditions() + var errs []error + err = controllerhelpers.RunSync( + &statusConditions, + namespaceControllerProgressingCondition, + namespaceControllerDegradedCondition, + nc.Generation, + func() ([]metav1.Condition, error) { + return ncc.syncNamespaces(ctx, nc, namespaces) + }, + ) + if err != nil { + errs = append(errs, fmt.Errorf("can't sync Namespace(s): %w", err)) + } + + err = controllerhelpers.RunSync( + &statusConditions, + clusterRoleControllerProgressingCondition, + clusterRoleControllerDegradedCondition, + nc.Generation, + func() ([]metav1.Condition, error) { + return ncc.syncClusterRoles(ctx, nc, clusterRoles) + }, + ) + if err != nil { + errs = append(errs, fmt.Errorf("can't sync ClusterRole(s): %w", err)) + } - err = ncc.syncNamespaces(ctx, namespaces) + err = controllerhelpers.RunSync( + &statusConditions, + roleControllerProgressingCondition, + roleControllerDegradedCondition, + nc.Generation, + func() ([]metav1.Condition, error) { + return ncc.syncRoles(ctx, nc, roles) + }, + ) if err != nil { - errs = append(errs, fmt.Errorf("sync Namespace(s): %w", err)) + errs = append(errs, fmt.Errorf("can't sync Role(s): %w", err)) } - err = ncc.syncClusterRoles(ctx, clusterRoles) + err = controllerhelpers.RunSync( + &statusConditions, + serviceAccountControllerProgressingCondition, + serviceAccountControllerDegradedCondition, + nc.Generation, + func() ([]metav1.Condition, error) { + return ncc.syncServiceAccounts(ctx, nc, serviceAccounts) + }, + ) if err != nil { - errs = append(errs, fmt.Errorf("sync ClusterRole(s): %w", err)) + errs = append(errs, fmt.Errorf("can't sync ServiceAccount(s): %w", err)) } - err = ncc.syncRoles(ctx, nc, roles) + err = controllerhelpers.RunSync( + &statusConditions, + clusterRoleBindingControllerProgressingCondition, + clusterRoleBindingControllerDegradedCondition, + nc.Generation, + func() ([]metav1.Condition, error) { + return ncc.syncClusterRoleBindings(ctx, nc, clusterRoleBindings) + }, + ) if err != nil { - errs = append(errs, fmt.Errorf("sync Role(s): %w", err)) + errs = append(errs, fmt.Errorf("can't sync ClusterRoleBinding(s): %w", err)) } - err = ncc.syncServiceAccounts(ctx, serviceAccounts) + err = controllerhelpers.RunSync( + &statusConditions, + roleBindingControllerProgressingCondition, + roleBindingControllerDegradedCondition, + nc.Generation, + func() ([]metav1.Condition, error) { + return ncc.syncRoleBindings(ctx, nc, roleBindings) + }, + ) if err != nil { - errs = append(errs, fmt.Errorf("sync ServiceAccount(s): %w", err)) + errs = append(errs, fmt.Errorf("can't sync RoleBinding(s): %w", err)) } - err = ncc.syncClusterRoleBindings(ctx, clusterRoleBindings) + err = controllerhelpers.RunSync( + &statusConditions, + daemonSetControllerProgressingCondition, + daemonSetControllerDegradedCondition, + nc.Generation, + func() ([]metav1.Condition, error) { + return ncc.syncDaemonSet(ctx, nc, soc, daemonSets) + }, + ) if err != nil { - errs = append(errs, fmt.Errorf("sync ClusterRoleBinding(s): %w", err)) + errs = append(errs, fmt.Errorf("can't sync DaemonSet(s): %w", err)) } - err = ncc.syncRoleBindings(ctx, nc, roleBindings) + matchingNodes, err := ncc.getMatchingNodes(nc) + if err != nil { + errs = append(errs, fmt.Errorf("can't get matching Nodes: %w", err)) + return utilerrors.NewAggregate(errs) + } + + // Aggregate conditions. + var aggregationErrs []error + + availableConditions := controllerhelpers.FindStatusConditionsWithSuffix(statusConditions, scyllav1alpha1.AvailableCondition) + + // Use a default available node condition for nodes which haven't propagated their conditions to status yet. + nodeAvailableConditionTypeFunc := func(node *corev1.Node) string { + return fmt.Sprintf(internalapi.NodeAvailableConditionFormat, node.Name) + } + defaultNodeAvailableConditions := makeDefaultNodeConditions( + nc.Generation, + matchingNodes, + availableConditions, + nodeAvailableConditionTypeFunc, + func(node *corev1.Node) metav1.Condition { + return metav1.Condition{ + Type: nodeAvailableConditionTypeFunc(node), + Status: metav1.ConditionFalse, + ObservedGeneration: nc.Generation, + Reason: internalapi.AwaitingConditionReason, + Message: fmt.Sprintf("Awaiting Available condition of node %q to be set.", naming.ObjRef(node)), + } + }, + ) + availableConditions = append(availableConditions, defaultNodeAvailableConditions...) + availableCondition, err := controllerhelpers.AggregateStatusConditions( + availableConditions, + metav1.Condition{ + Type: scyllav1alpha1.AvailableCondition, + Status: metav1.ConditionTrue, + Reason: internalapi.AsExpectedReason, + Message: "", + ObservedGeneration: nc.Generation, + }, + ) + if err != nil { + aggregationErrs = append(aggregationErrs, fmt.Errorf("can't aggregate available status conditions: %w", err)) + } + + progressingConditions := controllerhelpers.FindStatusConditionsWithSuffix(statusConditions, scyllav1alpha1.ProgressingCondition) + + // Use a default progressing node condition for nodes which haven't propagated their conditions to status yet. + nodeProgressingConditionTypeFunc := func(node *corev1.Node) string { + return fmt.Sprintf(internalapi.NodeProgressingConditionFormat, node.Name) + } + defaultNodeProgressingConditions := makeDefaultNodeConditions( + nc.Generation, + matchingNodes, + progressingConditions, + nodeProgressingConditionTypeFunc, + func(node *corev1.Node) metav1.Condition { + return metav1.Condition{ + Type: nodeProgressingConditionTypeFunc(node), + Status: metav1.ConditionTrue, + ObservedGeneration: nc.Generation, + Reason: internalapi.AwaitingConditionReason, + Message: fmt.Sprintf("Awaiting Progressing condition of node %q to be set.", naming.ObjRef(node)), + } + }, + ) + progressingConditions = append(progressingConditions, defaultNodeProgressingConditions...) + progressingCondition, err := controllerhelpers.AggregateStatusConditions( + progressingConditions, + metav1.Condition{ + Type: scyllav1alpha1.ProgressingCondition, + Status: metav1.ConditionFalse, + Reason: internalapi.AsExpectedReason, + Message: "", + ObservedGeneration: nc.Generation, + }, + ) if err != nil { - errs = append(errs, fmt.Errorf("sync RoleBinding(s): %w", err)) + aggregationErrs = append(aggregationErrs, fmt.Errorf("can't aggregate progressing status conditions: %w", err)) } - err = ncc.syncDaemonSet(ctx, nc, soc, daemonSets) + degradedCondition, err := controllerhelpers.AggregateStatusConditions( + controllerhelpers.FindStatusConditionsWithSuffix(statusConditions, scyllav1alpha1.DegradedCondition), + metav1.Condition{ + Type: scyllav1alpha1.DegradedCondition, + Status: metav1.ConditionFalse, + Reason: internalapi.AsExpectedReason, + Message: "", + ObservedGeneration: nc.Generation, + }, + ) if err != nil { - errs = append(errs, fmt.Errorf("sync DaemonSet(s): %w", err)) + aggregationErrs = append(aggregationErrs, fmt.Errorf("can't aggregate degraded status conditions: %w", err)) + } + + if len(aggregationErrs) > 0 { + errs = append(errs, aggregationErrs...) + return utilerrors.NewAggregate(errs) } + apimeta.SetStatusCondition(&statusConditions, availableCondition) + apimeta.SetStatusCondition(&statusConditions, progressingCondition) + apimeta.SetStatusCondition(&statusConditions, degradedCondition) + + // TODO(rzetelskik): remove NodeConfigReconciledConditionType in next API version. + reconciledCondition := getNodeConfigReconciledCondition(statusConditions, nc.Generation) + apimeta.SetStatusCondition(&statusConditions, reconciledCondition) + + status.Conditions = scyllav1alpha1.NewNodeConfigConditions(statusConditions) err = ncc.updateStatus(ctx, nc, status) - errs = append(errs, err) + if err != nil { + errs = append(errs, fmt.Errorf("can't update status: %w", err)) + } return utilerrors.NewAggregate(errs) } @@ -250,3 +419,67 @@ func (ncc *Controller) getServiceAccounts() (map[string]*corev1.ServiceAccount, return sasMap, nil } + +func (ncc *Controller) getMatchingNodes(nc *scyllav1alpha1.NodeConfig) ([]*corev1.Node, error) { + nodes, err := ncc.nodeLister.List(labels.Everything()) + if err != nil { + return nil, fmt.Errorf("can't list Nodes: %w", err) + } + + var errs []error + var matchingNodes []*corev1.Node + for _, n := range nodes { + isNodeConfigSelectingNode, err := controllerhelpers.IsNodeConfigSelectingNode(nc, n) + if err != nil { + errs = append(errs, fmt.Errorf("can't determine if NodeConfig %q is selecting Node %q: %w", naming.ObjRef(nc), naming.ObjRef(n), err)) + } + + if isNodeConfigSelectingNode { + matchingNodes = append(matchingNodes, n) + } + } + + err = utilerrors.NewAggregate(errs) + if err != nil { + return nil, err + } + + return matchingNodes, nil +} + +func makeDefaultNodeConditions(nodeConfigGeneration int64, nodes []*corev1.Node, conditions []metav1.Condition, nodeConditionTypeFunc func(*corev1.Node) string, defaultNodeConditionFunc func(*corev1.Node) metav1.Condition) []metav1.Condition { + var defaultNodeConditions []metav1.Condition + + for _, n := range nodes { + nodeConditionType := nodeConditionTypeFunc(n) + nodeCondition := apimeta.FindStatusCondition(conditions, nodeConditionType) + if nodeCondition == nil || nodeCondition.ObservedGeneration < nodeConfigGeneration { + defaultNodeConditions = append(defaultNodeConditions, defaultNodeConditionFunc(n)) + } + } + + return defaultNodeConditions +} + +func getNodeConfigReconciledCondition(conditions []metav1.Condition, generation int64) metav1.Condition { + reconciled := helpers.IsStatusConditionPresentAndTrue(conditions, scyllav1alpha1.AvailableCondition, generation) && + helpers.IsStatusConditionPresentAndFalse(conditions, scyllav1alpha1.ProgressingCondition, generation) && + helpers.IsStatusConditionPresentAndFalse(conditions, scyllav1alpha1.DegradedCondition, generation) + + reconciledCondition := metav1.Condition{ + Type: string(scyllav1alpha1.NodeConfigReconciledConditionType), + ObservedGeneration: generation, + } + + if reconciled { + reconciledCondition.Status = metav1.ConditionTrue + reconciledCondition.Reason = "FullyReconciledAndUp" + reconciledCondition.Message = "All operands are reconciled and available." + } else { + reconciledCondition.Status = metav1.ConditionFalse + reconciledCondition.Reason = "NotReconciledYet" + reconciledCondition.Message = "Not all operands are reconciled and available yet." + } + + return reconciledCondition +} diff --git a/pkg/controller/nodeconfig/sync_clusterrolebindings.go b/pkg/controller/nodeconfig/sync_clusterrolebindings.go index 5f38d2c2232..f7fb04b8415 100644 --- a/pkg/controller/nodeconfig/sync_clusterrolebindings.go +++ b/pkg/controller/nodeconfig/sync_clusterrolebindings.go @@ -6,6 +6,8 @@ import ( "context" "fmt" + scyllav1alpha1 "github.com/scylladb/scylla-operator/pkg/api/scylla/v1alpha1" + "github.com/scylladb/scylla-operator/pkg/controllerhelpers" "github.com/scylladb/scylla-operator/pkg/resourceapply" rbacv1 "k8s.io/api/rbac/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -53,27 +55,29 @@ func (ncc *Controller) pruneClusterRoleBindings(ctx context.Context, requiredClu return utilerrors.NewAggregate(errs) } -func (ncc *Controller) syncClusterRoleBindings( - ctx context.Context, - clusterRoleBindings map[string]*rbacv1.ClusterRoleBinding, -) error { +func (ncc *Controller) syncClusterRoleBindings(ctx context.Context, nc *scyllav1alpha1.NodeConfig, clusterRoleBindings map[string]*rbacv1.ClusterRoleBinding) ([]metav1.Condition, error) { + var progressingConditions []metav1.Condition + requiredClusterRoleBindings := ncc.makeClusterRoleBindings() // Delete any excessive ClusterRoleBindings. // Delete has to be the first action to avoid getting stuck on quota. if err := ncc.pruneClusterRoleBindings(ctx, requiredClusterRoleBindings, clusterRoleBindings); err != nil { - return fmt.Errorf("can't delete ClusterRoleBinding(s): %w", err) + return progressingConditions, fmt.Errorf("can't delete ClusterRoleBinding(s): %w", err) } var errs []error for _, crb := range requiredClusterRoleBindings { - _, _, err := resourceapply.ApplyClusterRoleBinding(ctx, ncc.kubeClient.RbacV1(), ncc.clusterRoleBindingLister, ncc.eventRecorder, crb, resourceapply.ApplyOptions{ + _, changed, err := resourceapply.ApplyClusterRoleBinding(ctx, ncc.kubeClient.RbacV1(), ncc.clusterRoleBindingLister, ncc.eventRecorder, crb, resourceapply.ApplyOptions{ AllowMissingControllerRef: true, }) + if changed { + controllerhelpers.AddGenericProgressingStatusCondition(&progressingConditions, clusterRoleBindingControllerProgressingCondition, crb, "apply", nc.Generation) + } if err != nil { errs = append(errs, fmt.Errorf("can't create missing clusterrole: %w", err)) continue } } - return utilerrors.NewAggregate(errs) + return progressingConditions, utilerrors.NewAggregate(errs) } diff --git a/pkg/controller/nodeconfig/sync_clusterroles.go b/pkg/controller/nodeconfig/sync_clusterroles.go index aea9412a0c5..8e3dfc7ccf6 100644 --- a/pkg/controller/nodeconfig/sync_clusterroles.go +++ b/pkg/controller/nodeconfig/sync_clusterroles.go @@ -6,6 +6,8 @@ import ( "context" "fmt" + scyllav1alpha1 "github.com/scylladb/scylla-operator/pkg/api/scylla/v1alpha1" + "github.com/scylladb/scylla-operator/pkg/controllerhelpers" "github.com/scylladb/scylla-operator/pkg/resourceapply" rbacv1 "k8s.io/api/rbac/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -53,25 +55,30 @@ func (ncc *Controller) pruneClusterRoles(ctx context.Context, requiredClusterRol return utilerrors.NewAggregate(errs) } -func (ncc *Controller) syncClusterRoles(ctx context.Context, clusterRoles map[string]*rbacv1.ClusterRole) error { +func (ncc *Controller) syncClusterRoles(ctx context.Context, nc *scyllav1alpha1.NodeConfig, clusterRoles map[string]*rbacv1.ClusterRole) ([]metav1.Condition, error) { + var progressingConditions []metav1.Condition + requiredClusterRoles := ncc.makeClusterRoles() // Delete any excessive ClusterRoles. // Delete has to be the first action to avoid getting stuck on quota. if err := ncc.pruneClusterRoles(ctx, requiredClusterRoles, clusterRoles); err != nil { - return fmt.Errorf("can't delete ClusterRole(s): %w", err) + return progressingConditions, fmt.Errorf("can't delete ClusterRole(s): %w", err) } var errs []error for _, cr := range requiredClusterRoles { - _, _, err := resourceapply.ApplyClusterRole(ctx, ncc.kubeClient.RbacV1(), ncc.clusterRoleLister, ncc.eventRecorder, cr, resourceapply.ApplyOptions{ + _, changed, err := resourceapply.ApplyClusterRole(ctx, ncc.kubeClient.RbacV1(), ncc.clusterRoleLister, ncc.eventRecorder, cr, resourceapply.ApplyOptions{ AllowMissingControllerRef: true, }) + if changed { + controllerhelpers.AddGenericProgressingStatusCondition(&progressingConditions, clusterRoleControllerProgressingCondition, cr, "apply", nc.Generation) + } if err != nil { errs = append(errs, fmt.Errorf("can't create missing clusterrole: %w", err)) continue } } - return utilerrors.NewAggregate(errs) + return progressingConditions, utilerrors.NewAggregate(errs) } diff --git a/pkg/controller/nodeconfig/sync_daemonsets.go b/pkg/controller/nodeconfig/sync_daemonsets.go index a2914c9166a..bbd2c34c95c 100644 --- a/pkg/controller/nodeconfig/sync_daemonsets.go +++ b/pkg/controller/nodeconfig/sync_daemonsets.go @@ -13,6 +13,8 @@ import ( "github.com/scylladb/scylla-operator/pkg/resourceapply" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + utilerrors "k8s.io/apimachinery/pkg/util/errors" ) func (ncc *Controller) syncDaemonSet( @@ -20,10 +22,12 @@ func (ncc *Controller) syncDaemonSet( nc *scyllav1alpha1.NodeConfig, soc *scyllav1alpha1.ScyllaOperatorConfig, daemonSets map[string]*appsv1.DaemonSet, -) error { +) ([]metav1.Condition, error) { + var progressingConditions []metav1.Condition + if soc.Status.ScyllaDBUtilsImage == nil || len(*soc.Status.ScyllaDBUtilsImage) == 0 { ncc.eventRecorder.Event(nc, corev1.EventTypeNormal, "MissingScyllaUtilsImage", "ScyllaOperatorConfig doesn't yet have scyllaUtilsImage available in the status.") - return controllertools.NewNonRetriable("scylla operator config doesn't yet have scyllaUtilsImage available in the status") + return progressingConditions, controllertools.NewNonRetriable("scylla operator config doesn't yet have scyllaUtilsImage available in the status") } scyllaDBUtilsImage := *soc.Status.ScyllaDBUtilsImage @@ -40,19 +44,39 @@ func (ncc *Controller) syncDaemonSet( }, ncc.eventRecorder) if err != nil { - return fmt.Errorf("can't prune DaemonSet(s): %w", err) + return progressingConditions, fmt.Errorf("can't prune DaemonSet(s): %w", err) } - for _, requiredDaemonSet := range requiredDaemonSets { - if requiredDaemonSet == nil { + var errs []error + for _, ds := range requiredDaemonSets { + if ds == nil { + continue + } + + updated, changed, err := resourceapply.ApplyDaemonSet(ctx, ncc.kubeClient.AppsV1(), ncc.daemonSetLister, ncc.eventRecorder, ds, resourceapply.ApplyOptions{}) + if changed { + controllerhelpers.AddGenericProgressingStatusCondition(&progressingConditions, daemonSetControllerProgressingCondition, ds, "apply", nc.Generation) + } + if err != nil { + errs = append(errs, fmt.Errorf("can't apply daemonset: %w", err)) continue } - _, _, err := resourceapply.ApplyDaemonSet(ctx, ncc.kubeClient.AppsV1(), ncc.daemonSetLister, ncc.eventRecorder, requiredDaemonSet, resourceapply.ApplyOptions{}) + rolledOut, err := controllerhelpers.IsDaemonSetRolledOut(updated) if err != nil { - return fmt.Errorf("can't apply daemonset: %w", err) + errs = append(errs, fmt.Errorf("can't check if daemonset is rolled out: %w", err)) + } + + if !rolledOut { + progressingConditions = append(progressingConditions, metav1.Condition{ + Type: daemonSetControllerProgressingCondition, + Status: metav1.ConditionTrue, + Reason: "WaitingForDaemonSetRollOut", + Message: fmt.Sprintf("Waiting for DaemonSet %q to roll out.", naming.ObjRef(ds)), + ObservedGeneration: nc.Generation, + }) } } - return nil + return progressingConditions, utilerrors.NewAggregate(errs) } diff --git a/pkg/controller/nodeconfig/sync_namespaces.go b/pkg/controller/nodeconfig/sync_namespaces.go index 092f0054747..b042029035c 100644 --- a/pkg/controller/nodeconfig/sync_namespaces.go +++ b/pkg/controller/nodeconfig/sync_namespaces.go @@ -6,6 +6,8 @@ import ( "context" "fmt" + scyllav1alpha1 "github.com/scylladb/scylla-operator/pkg/api/scylla/v1alpha1" + "github.com/scylladb/scylla-operator/pkg/controllerhelpers" "github.com/scylladb/scylla-operator/pkg/resourceapply" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -53,25 +55,30 @@ func (ncc *Controller) pruneNamespaces(ctx context.Context, requiredNamespaces [ return utilerrors.NewAggregate(errs) } -func (ncc *Controller) syncNamespaces(ctx context.Context, namespaces map[string]*corev1.Namespace) error { +func (ncc *Controller) syncNamespaces(ctx context.Context, nc *scyllav1alpha1.NodeConfig, namespaces map[string]*corev1.Namespace) ([]metav1.Condition, error) { + var progressingConditions []metav1.Condition + requiredNamespaces := ncc.makeNamespaces() // Delete any excessive Namespaces. // Delete has to be the first action to avoid getting stuck on quota. err := ncc.pruneNamespaces(ctx, requiredNamespaces, namespaces) if err != nil { - return fmt.Errorf("can't delete Namespace(s): %w", err) + return progressingConditions, fmt.Errorf("can't delete Namespace(s): %w", err) } var errs []error for _, ns := range requiredNamespaces { - _, _, err := resourceapply.ApplyNamespace(ctx, ncc.kubeClient.CoreV1(), ncc.namespaceLister, ncc.eventRecorder, ns, resourceapply.ApplyOptions{ + _, changed, err := resourceapply.ApplyNamespace(ctx, ncc.kubeClient.CoreV1(), ncc.namespaceLister, ncc.eventRecorder, ns, resourceapply.ApplyOptions{ AllowMissingControllerRef: true, }) + if changed { + controllerhelpers.AddGenericProgressingStatusCondition(&progressingConditions, namespaceControllerProgressingCondition, ns, "apply", nc.Generation) + } if err != nil { errs = append(errs, fmt.Errorf("can't create missing Namespace: %w", err)) continue } } - return utilerrors.NewAggregate(errs) + return progressingConditions, utilerrors.NewAggregate(errs) } diff --git a/pkg/controller/nodeconfig/sync_rolebindings.go b/pkg/controller/nodeconfig/sync_rolebindings.go index 67beff2b0ce..cc53a0f7e62 100644 --- a/pkg/controller/nodeconfig/sync_rolebindings.go +++ b/pkg/controller/nodeconfig/sync_rolebindings.go @@ -10,6 +10,7 @@ import ( "github.com/scylladb/scylla-operator/pkg/controllerhelpers" "github.com/scylladb/scylla-operator/pkg/resourceapply" rbacv1 "k8s.io/api/rbac/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" utilerrors "k8s.io/apimachinery/pkg/util/errors" ) @@ -17,7 +18,9 @@ func (ncc *Controller) syncRoleBindings( ctx context.Context, nc *scyllav1alpha1.NodeConfig, roleBindings map[string]*rbacv1.RoleBinding, -) error { +) ([]metav1.Condition, error) { + var progressingConditions []metav1.Condition + requiredRoleBindings := []*rbacv1.RoleBinding{ makePerftuneRoleBinding(), } @@ -33,18 +36,21 @@ func (ncc *Controller) syncRoleBindings( }, ncc.eventRecorder) if err != nil { - return fmt.Errorf("can't prune RoleBinding(s): %w", err) + return progressingConditions, fmt.Errorf("can't prune RoleBinding(s): %w", err) } var errs []error - for _, crb := range requiredRoleBindings { - _, _, err := resourceapply.ApplyRoleBinding(ctx, ncc.kubeClient.RbacV1(), ncc.roleBindingLister, ncc.eventRecorder, crb, resourceapply.ApplyOptions{ + for _, rb := range requiredRoleBindings { + _, changed, err := resourceapply.ApplyRoleBinding(ctx, ncc.kubeClient.RbacV1(), ncc.roleBindingLister, ncc.eventRecorder, rb, resourceapply.ApplyOptions{ AllowMissingControllerRef: true, }) + if changed { + controllerhelpers.AddGenericProgressingStatusCondition(&progressingConditions, roleBindingControllerProgressingCondition, rb, "apply", nc.Generation) + } if err != nil { errs = append(errs, fmt.Errorf("can't create missing rolebinding: %w", err)) continue } } - return utilerrors.NewAggregate(errs) + return progressingConditions, utilerrors.NewAggregate(errs) } diff --git a/pkg/controller/nodeconfig/sync_roles.go b/pkg/controller/nodeconfig/sync_roles.go index b4010329680..f973c631385 100644 --- a/pkg/controller/nodeconfig/sync_roles.go +++ b/pkg/controller/nodeconfig/sync_roles.go @@ -10,10 +10,13 @@ import ( "github.com/scylladb/scylla-operator/pkg/controllerhelpers" "github.com/scylladb/scylla-operator/pkg/resourceapply" rbacv1 "k8s.io/api/rbac/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" utilerrors "k8s.io/apimachinery/pkg/util/errors" ) -func (ncc *Controller) syncRoles(ctx context.Context, nc *scyllav1alpha1.NodeConfig, roles map[string]*rbacv1.Role) error { +func (ncc *Controller) syncRoles(ctx context.Context, nc *scyllav1alpha1.NodeConfig, roles map[string]*rbacv1.Role) ([]metav1.Condition, error) { + var progressingConditions []metav1.Condition + requiredRoles := []*rbacv1.Role{ makePerftuneRole(), } @@ -29,19 +32,22 @@ func (ncc *Controller) syncRoles(ctx context.Context, nc *scyllav1alpha1.NodeCon }, ncc.eventRecorder) if err != nil { - return fmt.Errorf("can't prune Role(s): %w", err) + return progressingConditions, fmt.Errorf("can't prune Role(s): %w", err) } var errs []error - for _, cr := range requiredRoles { - _, _, err := resourceapply.ApplyRole(ctx, ncc.kubeClient.RbacV1(), ncc.roleLister, ncc.eventRecorder, cr, resourceapply.ApplyOptions{ + for _, r := range requiredRoles { + _, changed, err := resourceapply.ApplyRole(ctx, ncc.kubeClient.RbacV1(), ncc.roleLister, ncc.eventRecorder, r, resourceapply.ApplyOptions{ AllowMissingControllerRef: true, }) + if changed { + controllerhelpers.AddGenericProgressingStatusCondition(&progressingConditions, roleControllerProgressingCondition, r, "apply", nc.Generation) + } if err != nil { errs = append(errs, fmt.Errorf("can't create missing Role: %w", err)) continue } } - return utilerrors.NewAggregate(errs) + return progressingConditions, utilerrors.NewAggregate(errs) } diff --git a/pkg/controller/nodeconfig/sync_serviceaccounts.go b/pkg/controller/nodeconfig/sync_serviceaccounts.go index 33611f35a8a..6fd41154991 100644 --- a/pkg/controller/nodeconfig/sync_serviceaccounts.go +++ b/pkg/controller/nodeconfig/sync_serviceaccounts.go @@ -6,6 +6,8 @@ import ( "context" "fmt" + scyllav1alpha1 "github.com/scylladb/scylla-operator/pkg/api/scylla/v1alpha1" + "github.com/scylladb/scylla-operator/pkg/controllerhelpers" "github.com/scylladb/scylla-operator/pkg/resourceapply" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -54,28 +56,30 @@ func (ncc *Controller) pruneServiceAccounts(ctx context.Context, requiredService return utilerrors.NewAggregate(errs) } -func (ncc *Controller) syncServiceAccounts( - ctx context.Context, - serviceAccounts map[string]*corev1.ServiceAccount, -) error { +func (ncc *Controller) syncServiceAccounts(ctx context.Context, nc *scyllav1alpha1.NodeConfig, serviceAccounts map[string]*corev1.ServiceAccount) ([]metav1.Condition, error) { + var progressingConditions []metav1.Condition + requiredServiceAccounts := ncc.makeServiceAccounts() // Delete any excessive ServiceAccounts. // Delete has to be the first action to avoid getting stuck on quota. if err := ncc.pruneServiceAccounts(ctx, requiredServiceAccounts, serviceAccounts); err != nil { - return fmt.Errorf("can't delete ServiceAccount(s): %w", err) + return progressingConditions, fmt.Errorf("can't delete ServiceAccount(s): %w", err) } var errs []error for _, sa := range requiredServiceAccounts { - _, _, err := resourceapply.ApplyServiceAccount(ctx, ncc.kubeClient.CoreV1(), ncc.serviceAccountLister, ncc.eventRecorder, sa, resourceapply.ApplyOptions{ + _, changed, err := resourceapply.ApplyServiceAccount(ctx, ncc.kubeClient.CoreV1(), ncc.serviceAccountLister, ncc.eventRecorder, sa, resourceapply.ApplyOptions{ AllowMissingControllerRef: true, }) + if changed { + controllerhelpers.AddGenericProgressingStatusCondition(&progressingConditions, serviceAccountControllerProgressingCondition, sa, "apply", nc.Generation) + } if err != nil { errs = append(errs, fmt.Errorf("can't create missing ServiceAccount: %w", err)) continue } } - return utilerrors.NewAggregate(errs) + return progressingConditions, utilerrors.NewAggregate(errs) } diff --git a/pkg/controller/nodesetup/status.go b/pkg/controller/nodesetup/status.go index e8408fb3226..cfbe8161b8c 100644 --- a/pkg/controller/nodesetup/status.go +++ b/pkg/controller/nodesetup/status.go @@ -7,40 +7,26 @@ import ( "fmt" "github.com/scylladb/scylla-operator/pkg/api/scylla/v1alpha1" - "github.com/scylladb/scylla-operator/pkg/controllerhelpers" - corev1 "k8s.io/api/core/v1" apiequality "k8s.io/apimachinery/pkg/api/equality" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/klog/v2" ) -func convertMetaToNodeConfigConditions(conditions []metav1.Condition) []v1alpha1.NodeConfigCondition { - converted := make([]v1alpha1.NodeConfigCondition, 0, len(conditions)) - for _, c := range conditions { - converted = append(converted, v1alpha1.NodeConfigCondition{ - Type: v1alpha1.NodeConfigConditionType(c.Type), - Status: corev1.ConditionStatus(c.Status), - ObservedGeneration: c.ObservedGeneration, - LastTransitionTime: c.LastTransitionTime, - Reason: c.Reason, - Message: c.Message, - }) - } +func (nsc *Controller) calculateStatus(nc *v1alpha1.NodeConfig) *v1alpha1.NodeConfigStatus { + status := nc.Status.DeepCopy() + status.ObservedGeneration = nc.Generation - return converted + return status } -func (nsc *Controller) updateNodeStatus(ctx context.Context, currentNC *v1alpha1.NodeConfig, conditions []metav1.Condition) error { - nc := currentNC.DeepCopy() - - for _, cond := range convertMetaToNodeConfigConditions(conditions) { - controllerhelpers.SetNodeConfigStatusCondition(&nc.Status.Conditions, cond) - } - - if apiequality.Semantic.DeepEqual(nc.Status, currentNC.Status) { +func (nsc *Controller) updateStatus(ctx context.Context, currentNC *v1alpha1.NodeConfig, status *v1alpha1.NodeConfigStatus) error { + if apiequality.Semantic.DeepEqual(currentNC.Status, status) { return nil } + nc := currentNC.DeepCopy() + nc.Status = *status + klog.V(2).InfoS("Updating status", "NodeConfig", klog.KObj(currentNC), "Node", nsc.nodeName) _, err := nsc.scyllaClient.NodeConfigs().UpdateStatus(ctx, nc, metav1.UpdateOptions{}) diff --git a/pkg/controller/nodesetup/sync.go b/pkg/controller/nodesetup/sync.go index 27020c359a3..4e2e0295f2a 100644 --- a/pkg/controller/nodesetup/sync.go +++ b/pkg/controller/nodesetup/sync.go @@ -7,8 +7,11 @@ import ( "fmt" "time" + scyllav1alpha1 "github.com/scylladb/scylla-operator/pkg/api/scylla/v1alpha1" "github.com/scylladb/scylla-operator/pkg/controllerhelpers" + "github.com/scylladb/scylla-operator/pkg/internalapi" apierrors "k8s.io/apimachinery/pkg/api/errors" + apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/klog/v2" @@ -34,15 +37,17 @@ func (nsc *Controller) sync(ctx context.Context) error { return fmt.Errorf("nodeConfig UID %q doesn't match the expected UID %q", nc.UID, nc.UID) } - var errs []error - var conditions []metav1.Condition + status := nsc.calculateStatus(nc) if nc.DeletionTimestamp != nil { - return nsc.updateNodeStatus(ctx, nc, conditions) + return nsc.updateStatus(ctx, nc, status) } + statusConditions := status.Conditions.ToMetaV1Conditions() + + var errs []error err = controllerhelpers.RunSync( - &conditions, + &statusConditions, fmt.Sprintf(loopDeviceControllerNodeProgressingConditionFormat, nsc.nodeName), fmt.Sprintf(loopDeviceControllerNodeDegradedConditionFormat, nsc.nodeName), nc.Generation, @@ -55,7 +60,7 @@ func (nsc *Controller) sync(ctx context.Context) error { } err = controllerhelpers.RunSync( - &conditions, + &statusConditions, fmt.Sprintf(raidControllerNodeProgressingConditionFormat, nsc.nodeName), fmt.Sprintf(raidControllerNodeDegradedConditionFormat, nsc.nodeName), nc.Generation, @@ -68,7 +73,7 @@ func (nsc *Controller) sync(ctx context.Context) error { } err = controllerhelpers.RunSync( - &conditions, + &statusConditions, fmt.Sprintf(filesystemControllerNodeProgressingConditionFormat, nsc.nodeName), fmt.Sprintf(filesystemControllerNodeDegradedConditionFormat, nsc.nodeName), nc.Generation, @@ -81,7 +86,7 @@ func (nsc *Controller) sync(ctx context.Context) error { } err = controllerhelpers.RunSync( - &conditions, + &statusConditions, fmt.Sprintf(mountControllerNodeProgressingConditionFormat, nsc.nodeName), fmt.Sprintf(mountControllerNodeDegradedConditionFormat, nsc.nodeName), nc.Generation, @@ -93,20 +98,67 @@ func (nsc *Controller) sync(ctx context.Context) error { errs = append(errs, fmt.Errorf("can't sync mounts: %w", err)) } - err = controllerhelpers.SetAggregatedNodeConditions(nsc.nodeName, &conditions, nc.Generation) + // Aggregate node conditions. + var aggregationErrs []error + nodeAvailableConditionType := fmt.Sprintf(internalapi.NodeAvailableConditionFormat, nsc.nodeName) + nodeAvailableCondition, err := controllerhelpers.AggregateStatusConditions( + controllerhelpers.FindStatusConditionsWithSuffix(statusConditions, nodeAvailableConditionType), + metav1.Condition{ + Type: nodeAvailableConditionType, + Status: metav1.ConditionTrue, + Reason: internalapi.AsExpectedReason, + Message: "", + ObservedGeneration: nc.Generation, + }, + ) if err != nil { - errs = append(errs, fmt.Errorf("can't aggregate conditions: %w", err)) + aggregationErrs = append(aggregationErrs, fmt.Errorf("can't aggregate available node status conditions: %w", err)) } - err = nsc.updateNodeStatus(ctx, nc, conditions) + nodeProgressingConditionType := fmt.Sprintf(internalapi.NodeProgressingConditionFormat, nsc.nodeName) + nodeProgressingCondition, err := controllerhelpers.AggregateStatusConditions( + controllerhelpers.FindStatusConditionsWithSuffix(statusConditions, nodeProgressingConditionType), + metav1.Condition{ + Type: nodeProgressingConditionType, + Status: metav1.ConditionFalse, + Reason: internalapi.AsExpectedReason, + Message: "", + ObservedGeneration: nc.Generation, + }, + ) if err != nil { - errs = append(errs, fmt.Errorf("can't update node status: %w", err)) + aggregationErrs = append(aggregationErrs, fmt.Errorf("can't aggregate progressing node status conditions: %w", err)) } - err = utilerrors.NewAggregate(errs) + nodeDegradedConditionType := fmt.Sprintf(internalapi.NodeDegradedConditionFormat, nsc.nodeName) + nodeDegradedCondition, err := controllerhelpers.AggregateStatusConditions( + controllerhelpers.FindStatusConditionsWithSuffix(statusConditions, nodeDegradedConditionType), + metav1.Condition{ + Type: nodeDegradedConditionType, + Status: metav1.ConditionFalse, + Reason: internalapi.AsExpectedReason, + Message: "", + ObservedGeneration: nc.Generation, + }, + ) + if err != nil { + aggregationErrs = append(aggregationErrs, fmt.Errorf("can't aggregate degraded node status conditions: %w", err)) + } + + if len(aggregationErrs) > 0 { + errs = append(errs, aggregationErrs...) + return utilerrors.NewAggregate(errs) + } + + apimeta.SetStatusCondition(&statusConditions, nodeAvailableCondition) + apimeta.SetStatusCondition(&statusConditions, nodeProgressingCondition) + apimeta.SetStatusCondition(&statusConditions, nodeDegradedCondition) + + status.Conditions = scyllav1alpha1.NewNodeConfigConditions(statusConditions) + err = nsc.updateStatus(ctx, nc, status) if err != nil { - return fmt.Errorf("failed to sync nodeconfig %q: %w", nsc.nodeConfigName, err) + errs = append(errs, fmt.Errorf("can't update status: %w", err)) } - return nil + return utilerrors.NewAggregate(errs) } diff --git a/pkg/controllerhelpers/scylla.go b/pkg/controllerhelpers/scylla.go index 8056ea40aa4..29d530442fa 100644 --- a/pkg/controllerhelpers/scylla.go +++ b/pkg/controllerhelpers/scylla.go @@ -3,16 +3,12 @@ package controllerhelpers import ( "fmt" "sort" - "time" scyllav1 "github.com/scylladb/scylla-operator/pkg/api/scylla/v1" scyllav1alpha1 "github.com/scylladb/scylla-operator/pkg/api/scylla/v1alpha1" - "github.com/scylladb/scylla-operator/pkg/internalapi" "github.com/scylladb/scylla-operator/pkg/naming" "github.com/scylladb/scylla-operator/pkg/scyllaclient" corev1 "k8s.io/api/core/v1" - apimeta "k8s.io/apimachinery/pkg/api/meta" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/util/errors" corev1listers "k8s.io/client-go/listers/core/v1" @@ -166,108 +162,6 @@ func SetNodeStatus(nodeStatuses []scyllav1alpha1.NodeConfigNodeStatus, status *s return nodeStatuses } -func SetAggregatedNodeConditions(nodeName string, conditions *[]metav1.Condition, generation int64) error { - const ( - nodeAvailableConditionFormat = "Node%sAvailable" - nodeProgressingConditionFormat = "Node%sProgressing" - nodeDegradedConditionFormat = "Node%sDegraded" - ) - - nodeAvailableConditionType := fmt.Sprintf(nodeAvailableConditionFormat, nodeName) - availableCondition, err := AggregateStatusConditions( - FindStatusConditionsWithSuffix(*conditions, nodeAvailableConditionType), - metav1.Condition{ - Type: nodeAvailableConditionType, - Status: metav1.ConditionTrue, - Reason: internalapi.AsExpectedReason, - Message: "", - ObservedGeneration: generation, - }, - ) - if err != nil { - return fmt.Errorf("can't aggregate status conditions: %w", err) - } - apimeta.SetStatusCondition(conditions, availableCondition) - - nodeProgressingConditionType := fmt.Sprintf(nodeProgressingConditionFormat, nodeName) - progressingCondition, err := AggregateStatusConditions( - FindStatusConditionsWithSuffix(*conditions, nodeProgressingConditionType), - metav1.Condition{ - Type: nodeProgressingConditionType, - Status: metav1.ConditionFalse, - Reason: internalapi.AsExpectedReason, - Message: "", - ObservedGeneration: generation, - }, - ) - if err != nil { - return fmt.Errorf("can't aggregate status conditions: %w", err) - } - apimeta.SetStatusCondition(conditions, progressingCondition) - - nodeDegradedConditionType := fmt.Sprintf(nodeDegradedConditionFormat, nodeName) - degradedCondition, err := AggregateStatusConditions( - FindStatusConditionsWithSuffix(*conditions, nodeDegradedConditionType), - metav1.Condition{ - Type: nodeDegradedConditionType, - Status: metav1.ConditionFalse, - Reason: internalapi.AsExpectedReason, - Message: "", - ObservedGeneration: generation, - }, - ) - if err != nil { - return fmt.Errorf("can't aggregate status conditions: %w", err) - } - apimeta.SetStatusCondition(conditions, degradedCondition) - - return nil -} - -// SetNodeConfigStatusCondition sets the corresponding condition in conditions to newCondition. -// conditions must be non-nil. -// If the condition of the specified type already exists (all fields of the existing condition are updated to -// newCondition, LastTransitionTime is set to now if the new status differs from the old status) -// If a condition of the specified type does not exist (LastTransitionTime is set to now() if unset, and newCondition is appended) -func SetNodeConfigStatusCondition(conditions *[]scyllav1alpha1.NodeConfigCondition, newCondition scyllav1alpha1.NodeConfigCondition) { - if conditions == nil { - return - } - - existingCondition := FindNodeConfigCondition(*conditions, newCondition.Type) - if existingCondition == nil { - if newCondition.LastTransitionTime.IsZero() { - newCondition.LastTransitionTime = metav1.NewTime(time.Now()) - } - *conditions = append(*conditions, newCondition) - return - } - - if existingCondition.Status != newCondition.Status { - existingCondition.Status = newCondition.Status - if !newCondition.LastTransitionTime.IsZero() { - existingCondition.LastTransitionTime = newCondition.LastTransitionTime - } else { - existingCondition.LastTransitionTime = metav1.NewTime(time.Now()) - } - } - - existingCondition.Reason = newCondition.Reason - existingCondition.Message = newCondition.Message - existingCondition.ObservedGeneration = newCondition.ObservedGeneration -} - -// FindNodeConfigCondition finds the conditionType in conditions. -func FindNodeConfigCondition(conditions []scyllav1alpha1.NodeConfigCondition, conditionType scyllav1alpha1.NodeConfigConditionType) *scyllav1alpha1.NodeConfigCondition { - for i := range conditions { - if conditions[i].Type == conditionType { - return &conditions[i] - } - } - - return nil -} - func IsNodeConfigSelectingNode(nc *scyllav1alpha1.NodeConfig, node *corev1.Node) (bool, error) { // Check nodeSelector. diff --git a/pkg/controllerhelpers/scylla_test.go b/pkg/controllerhelpers/scylla_test.go index e8febecf856..37eb066a189 100644 --- a/pkg/controllerhelpers/scylla_test.go +++ b/pkg/controllerhelpers/scylla_test.go @@ -4,9 +4,7 @@ import ( "fmt" "reflect" "testing" - "time" - "github.com/google/go-cmp/cmp" scyllav1 "github.com/scylladb/scylla-operator/pkg/api/scylla/v1" scyllav1alpha1 "github.com/scylladb/scylla-operator/pkg/api/scylla/v1alpha1" corev1 "k8s.io/api/core/v1" @@ -168,157 +166,6 @@ func TestIsNodeConfigSelectingNode(t *testing.T) { } } -func TestSetNodeConfigStatusCondition(t *testing.T) { - now := metav1.Now() - old := metav1.NewTime(now.Add(-1 * time.Hour)) - - tt := []struct { - name string - existing []scyllav1alpha1.NodeConfigCondition - cond scyllav1alpha1.NodeConfigCondition - expected []scyllav1alpha1.NodeConfigCondition - }{ - { - name: "add new condition", - existing: nil, - cond: scyllav1alpha1.NodeConfigCondition{ - Type: scyllav1alpha1.NodeConfigReconciledConditionType, - Status: corev1.ConditionTrue, - Reason: "Up", - Message: "All good.", - LastTransitionTime: now, - }, - expected: []scyllav1alpha1.NodeConfigCondition{ - { - Type: scyllav1alpha1.NodeConfigReconciledConditionType, - Status: corev1.ConditionTrue, - Reason: "Up", - Message: "All good.", - LastTransitionTime: now, - }, - }, - }, - { - name: "update existing condition without an edge", - existing: []scyllav1alpha1.NodeConfigCondition{ - { - Type: scyllav1alpha1.NodeConfigReconciledConditionType, - Status: corev1.ConditionTrue, - Reason: "Up", - Message: "All good.", - LastTransitionTime: old, - }, - }, - cond: scyllav1alpha1.NodeConfigCondition{ - Type: scyllav1alpha1.NodeConfigReconciledConditionType, - Status: corev1.ConditionTrue, - Reason: "TotallyUp", - Message: "Even better.", - LastTransitionTime: now, - }, - expected: []scyllav1alpha1.NodeConfigCondition{ - { - Type: scyllav1alpha1.NodeConfigReconciledConditionType, - Status: corev1.ConditionTrue, - Reason: "TotallyUp", - Message: "Even better.", - LastTransitionTime: old, - }, - }, - }, - { - name: "update existing condition with an edge", - existing: []scyllav1alpha1.NodeConfigCondition{ - { - Type: scyllav1alpha1.NodeConfigReconciledConditionType, - Status: corev1.ConditionFalse, - Reason: "Down", - Message: "No pods available.", - LastTransitionTime: old, - }, - }, - cond: scyllav1alpha1.NodeConfigCondition{ - Type: scyllav1alpha1.NodeConfigReconciledConditionType, - Status: corev1.ConditionTrue, - Reason: "Up", - Message: "All good.", - LastTransitionTime: now, - }, - expected: []scyllav1alpha1.NodeConfigCondition{ - { - Type: scyllav1alpha1.NodeConfigReconciledConditionType, - Status: corev1.ConditionTrue, - Reason: "Up", - Message: "All good.", - LastTransitionTime: now, - }, - }, - }, - } - - for _, tc := range tt { - t.Run(tc.name, func(t *testing.T) { - status := &scyllav1alpha1.NodeConfigStatus{ - Conditions: tc.existing, - } - status = status.DeepCopy() - - SetNodeConfigStatusCondition(&status.Conditions, tc.cond) - - if !reflect.DeepEqual(status.Conditions, tc.expected) { - t.Errorf("expected and actual conditions differ: %s", cmp.Diff(tc.expected, status.Conditions)) - } - }) - } -} - -func TestFindNodeConfigCondition(t *testing.T) { - t.Parallel() - - tt := []struct { - name string - conditions []scyllav1alpha1.NodeConfigCondition - conditionType scyllav1alpha1.NodeConfigConditionType - expected *scyllav1alpha1.NodeConfigCondition - }{ - { - name: "no matching condition type", - conditions: []scyllav1alpha1.NodeConfigCondition{ - { - Type: scyllav1alpha1.AvailableCondition, - }, - }, - conditionType: scyllav1alpha1.DegradedCondition, - expected: nil, - }, - { - name: "no matching condition type", - conditions: []scyllav1alpha1.NodeConfigCondition{ - { - Type: scyllav1alpha1.AvailableCondition, - }, - { - Type: scyllav1alpha1.DegradedCondition, - }, - }, - conditionType: scyllav1alpha1.DegradedCondition, - expected: &scyllav1alpha1.NodeConfigCondition{Type: scyllav1alpha1.DegradedCondition}, - }, - } - - for _, tc := range tt { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - - actual := FindNodeConfigCondition(tc.conditions, tc.conditionType) - - if !reflect.DeepEqual(actual, tc.expected) { - t.Errorf("expected and actual conditions differ: %s", cmp.Diff(tc.expected, actual)) - } - }) - } -} - func TestGetScyllaHost(t *testing.T) { t.Parallel() diff --git a/pkg/internalapi/conditions.go b/pkg/internalapi/conditions.go index cd19abe232d..ab5e4dd5f2c 100644 --- a/pkg/internalapi/conditions.go +++ b/pkg/internalapi/conditions.go @@ -1,7 +1,12 @@ package internalapi const ( - AsExpectedReason = "AsExpected" - ErrorReason = "Error" - ProgressingReason = "Progressing" + NodeAvailableConditionFormat = "Node%sAvailable" + NodeProgressingConditionFormat = "Node%sProgressing" + NodeDegradedConditionFormat = "Node%sDegraded" + + AsExpectedReason = "AsExpected" + ErrorReason = "Error" + ProgressingReason = "Progressing" + AwaitingConditionReason = "AwaitingCondition" ) From 8b71353bc6173da684f3e428f3e56e3f8b473116 Mon Sep 17 00:00:00 2001 From: Kacper Rzetelski Date: Thu, 29 Aug 2024 16:58:53 +0200 Subject: [PATCH 2/3] Accomodate NodeConfig condition aggregation in e2e tests and add a test for degraded condition propagation --- .../set/nodeconfig/nodeconfig_disksetup.go | 75 ++++++++++++- .../nodeconfig/nodeconfig_optimizations.go | 1 - test/e2e/set/nodeconfig/verify.go | 106 +++++++++++++++--- test/e2e/utils/helpers.go | 58 +++------- 4 files changed, 180 insertions(+), 60 deletions(-) diff --git a/test/e2e/set/nodeconfig/nodeconfig_disksetup.go b/test/e2e/set/nodeconfig/nodeconfig_disksetup.go index 12fbfc2e76e..f1412abc945 100644 --- a/test/e2e/set/nodeconfig/nodeconfig_disksetup.go +++ b/test/e2e/set/nodeconfig/nodeconfig_disksetup.go @@ -14,6 +14,8 @@ import ( configassests "github.com/scylladb/scylla-operator/assets/config" scyllav1alpha1 "github.com/scylladb/scylla-operator/pkg/api/scylla/v1alpha1" "github.com/scylladb/scylla-operator/pkg/controllerhelpers" + "github.com/scylladb/scylla-operator/pkg/helpers" + "github.com/scylladb/scylla-operator/pkg/internalapi" "github.com/scylladb/scylla-operator/pkg/pointer" scyllafixture "github.com/scylladb/scylla-operator/test/e2e/fixture/scylla" "github.com/scylladb/scylla-operator/test/e2e/framework" @@ -183,10 +185,12 @@ var _ = g.Describe("Node Setup", framework.Serial, func() { nc.Name, controllerhelpers.WaitForStateOptions{TolerateDelete: false}, utils.IsNodeConfigRolledOut, - utils.IsNodeConfigDoneWithNodes(matchingNodes), + utils.IsNodeConfigDoneWithNodeTuningFunc(matchingNodes), ) o.Expect(err).NotTo(o.HaveOccurred()) + verifyNodeConfig(ctx, f.KubeAdminClient(), nc) + hostLoopsDir := "/host/dev/loops" framework.By("Checking if loop devices has been created at %q", hostLoopsDir) o.Eventually(func(g o.Gomega) { @@ -269,13 +273,80 @@ var _ = g.Describe("Node Setup", framework.Serial, func() { nc.Name, controllerhelpers.WaitForStateOptions{TolerateDelete: false}, utils.IsNodeConfigRolledOut, - utils.IsNodeConfigDoneWithNodes(matchingNodes), + utils.IsNodeConfigDoneWithNodeTuningFunc(matchingNodes), ) o.Expect(err).NotTo(o.HaveOccurred()) + + verifyNodeConfig(ctx, f.KubeAdminClient(), nc) }, g.Entry("out of one loop device", 1), g.Entry("out of three loop devices", 3), ) + + g.It("should propagate degraded conditions for invalid configuration", func() { + ctx, cancel := context.WithTimeout(context.Background(), testTimeout) + defer cancel() + + nc := ncTemplate.DeepCopy() + + nc.Spec.LocalDiskSetup = &scyllav1alpha1.LocalDiskSetup{ + Mounts: []scyllav1alpha1.MountConfiguration{ + { + Device: fmt.Sprintf("/dev/%s", f.Namespace()), + MountPoint: fmt.Sprintf("/mnt/%s", f.Namespace()), + FSType: string(scyllav1alpha1.XFSFilesystem), + UnsupportedOptions: []string{"prjquota"}, + }, + }, + } + + g.By("Creating NodeConfig") + nc, err := f.ScyllaAdminClient().ScyllaV1alpha1().NodeConfigs().Create(ctx, nc, metav1.CreateOptions{}) + o.Expect(err).NotTo(o.HaveOccurred()) + + isNodeConfigMountControllerDegraded := func(nc *scyllav1alpha1.NodeConfig) (bool, error) { + statusConditions := nc.Status.Conditions.ToMetaV1Conditions() + + if !helpers.IsStatusConditionPresentAndTrue(statusConditions, scyllav1alpha1.AvailableCondition, nc.Generation) { + return false, nil + } + + if !helpers.IsStatusConditionPresentAndFalse(statusConditions, scyllav1alpha1.ProgressingCondition, nc.Generation) { + return false, nil + } + + if !helpers.IsStatusConditionPresentAndTrue(statusConditions, scyllav1alpha1.DegradedCondition, nc.Generation) { + return false, nil + } + + for _, n := range matchingNodes { + nodeDegradedConditionType := fmt.Sprintf(internalapi.NodeDegradedConditionFormat, n.GetName()) + if !helpers.IsStatusConditionPresentAndTrue(statusConditions, nodeDegradedConditionType, nc.Generation) { + return false, nil + } + + nodeRaidControllerConditionType := fmt.Sprintf("MountControllerNode%sDegraded", n.GetName()) + if !helpers.IsStatusConditionPresentAndTrue(statusConditions, nodeRaidControllerConditionType, nc.Generation) { + return false, nil + } + } + + return true, nil + } + + framework.By("Waiting for NodeConfig to be in a degraded state") + ctx1, ctx1Cancel := context.WithTimeout(ctx, nodeConfigRolloutTimeout) + defer ctx1Cancel() + nc, err = controllerhelpers.WaitForNodeConfigState( + ctx1, + f.ScyllaAdminClient().ScyllaV1alpha1().NodeConfigs(), + nc.Name, + controllerhelpers.WaitForStateOptions{TolerateDelete: false}, + utils.IsNodeConfigDoneWithNodeTuningFunc(matchingNodes), + isNodeConfigMountControllerDegraded, + ) + o.Expect(err).NotTo(o.HaveOccurred()) + }) }) func executeInPod(config *rest.Config, client corev1client.CoreV1Interface, pod *corev1.Pod, command string, args ...string) (string, string, error) { diff --git a/test/e2e/set/nodeconfig/nodeconfig_optimizations.go b/test/e2e/set/nodeconfig/nodeconfig_optimizations.go index 39a19531b73..30e4e724442 100644 --- a/test/e2e/set/nodeconfig/nodeconfig_optimizations.go +++ b/test/e2e/set/nodeconfig/nodeconfig_optimizations.go @@ -81,7 +81,6 @@ var _ = g.Describe("NodeConfig Optimizations", framework.Serial, func() { controllerhelpers.WaitForStateOptions{TolerateDelete: false}, utils.IsNodeConfigRolledOut, utils.IsNodeConfigDoneWithNodeTuningFunc(matchingNodes), - utils.IsNodeConfigDoneWithNodes(matchingNodes), ) o.Expect(err).NotTo(o.HaveOccurred()) diff --git a/test/e2e/set/nodeconfig/verify.go b/test/e2e/set/nodeconfig/verify.go index 672f4b40fdf..34c7fa6b055 100644 --- a/test/e2e/set/nodeconfig/verify.go +++ b/test/e2e/set/nodeconfig/verify.go @@ -59,8 +59,82 @@ func verifyNodeConfig(ctx context.Context, kubeClient kubernetes.Interface, nc * condType string status corev1.ConditionStatus } + condList := []condValue{ + // Aggregated conditions + { + condType: "Available", + status: corev1.ConditionTrue, + }, + { + condType: "Progressing", + status: corev1.ConditionFalse, + }, + { + condType: "Degraded", + status: corev1.ConditionFalse, + }, + + // Controller conditions + { + condType: "NamespaceControllerProgressing", + status: corev1.ConditionFalse, + }, + { + condType: "NamespaceControllerDegraded", + status: corev1.ConditionFalse, + }, + { + condType: "ClusterRoleControllerProgressing", + status: corev1.ConditionFalse, + }, + { + condType: "ClusterRoleControllerDegraded", + status: corev1.ConditionFalse, + }, + { + condType: "ServiceAccountControllerProgressing", + status: corev1.ConditionFalse, + }, + { + condType: "ServiceAccountControllerDegraded", + status: corev1.ConditionFalse, + }, + { + condType: "ClusterRoleBindingControllerProgressing", + status: corev1.ConditionFalse, + }, + { + condType: "ClusterRoleBindingControllerDegraded", + status: corev1.ConditionFalse, + }, + { + condType: "DaemonSetControllerProgressing", + status: corev1.ConditionFalse, + }, + { + condType: "DaemonSetControllerDegraded", + status: corev1.ConditionFalse, + }, + { + condType: "RoleControllerProgressing", + status: corev1.ConditionFalse, + }, + { + condType: "RoleControllerDegraded", + status: corev1.ConditionFalse, + }, + { + condType: "RoleBindingControllerProgressing", + status: corev1.ConditionFalse, + }, + { + condType: "RoleBindingControllerDegraded", + status: corev1.ConditionFalse, + }, + } + for _, nodeName := range dsNodeNames { - condList := []condValue{ + nodeCondList := []condValue{ // Node aggregated conditions { condType: fmt.Sprintf("Node%sAvailable", nodeName), @@ -74,7 +148,7 @@ func verifyNodeConfig(ctx context.Context, kubeClient kubernetes.Interface, nc * condType: fmt.Sprintf("Node%sDegraded", nodeName), status: corev1.ConditionFalse, }, - // Controller conditions + // Node controller conditions { condType: fmt.Sprintf("FilesystemControllerNode%sProgressing", nodeName), status: corev1.ConditionFalse, @@ -109,25 +183,27 @@ func verifyNodeConfig(ctx context.Context, kubeClient kubernetes.Interface, nc * }, } - for _, item := range condList { - expectedConditions = append(expectedConditions, scyllav1alpha1.NodeConfigCondition{ - Type: scyllav1alpha1.NodeConfigConditionType(item.condType), - Status: item.status, - Reason: "AsExpected", - Message: "", - ObservedGeneration: nc.Generation, - }) - } + condList = append(condList, nodeCondList...) + } + for _, item := range condList { expectedConditions = append(expectedConditions, scyllav1alpha1.NodeConfigCondition{ - Type: scyllav1alpha1.NodeConfigReconciledConditionType, - Status: corev1.ConditionTrue, - Reason: "FullyReconciledAndUp", - Message: "All operands are reconciled and available.", + Type: scyllav1alpha1.NodeConfigConditionType(item.condType), + Status: item.status, + Reason: "AsExpected", + Message: "", ObservedGeneration: nc.Generation, }) } + expectedConditions = append(expectedConditions, scyllav1alpha1.NodeConfigCondition{ + Type: scyllav1alpha1.NodeConfigReconciledConditionType, + Status: corev1.ConditionTrue, + ObservedGeneration: nc.Generation, + Reason: "FullyReconciledAndUp", + Message: "All operands are reconciled and available.", + }) + return expectedConditions }()...)) } diff --git a/test/e2e/utils/helpers.go b/test/e2e/utils/helpers.go index 982213912ff..80adfd97f4d 100644 --- a/test/e2e/utils/helpers.go +++ b/test/e2e/utils/helpers.go @@ -40,9 +40,22 @@ import ( ) func IsNodeConfigRolledOut(nc *scyllav1alpha1.NodeConfig) (bool, error) { - cond := controllerhelpers.FindNodeConfigCondition(nc.Status.Conditions, scyllav1alpha1.NodeConfigReconciledConditionType) - return nc.Status.ObservedGeneration >= nc.Generation && - cond != nil && cond.ObservedGeneration >= nc.Generation && cond.Status == corev1.ConditionTrue, nil + statusConditions := nc.Status.Conditions.ToMetaV1Conditions() + + if !helpers.IsStatusConditionPresentAndTrue(statusConditions, scyllav1alpha1.AvailableCondition, nc.Generation) { + return false, nil + } + + if !helpers.IsStatusConditionPresentAndFalse(statusConditions, scyllav1alpha1.ProgressingCondition, nc.Generation) { + return false, nil + } + + if !helpers.IsStatusConditionPresentAndFalse(statusConditions, scyllav1alpha1.DegradedCondition, nc.Generation) { + return false, nil + } + + framework.Infof("NodeConfig %q (RV=%s) is rolled out", naming.ObjRef(nc), nc.ResourceVersion) + return true, nil } func GetMatchingNodesForNodeConfig(ctx context.Context, nodeGetter corev1client.NodesGetter, nc *scyllav1alpha1.NodeConfig) ([]*corev1.Node, error) { @@ -64,45 +77,6 @@ func GetMatchingNodesForNodeConfig(ctx context.Context, nodeGetter corev1client. return matchingNodes, nil } -func IsNodeConfigDoneWithNodes(nodes []*corev1.Node) func(nc *scyllav1alpha1.NodeConfig) (bool, error) { - const ( - nodeAvailableConditionFormat = "Node%sAvailable" - nodeProgressingConditionFormat = "Node%sProgressing" - nodeDegradedConditionFormat = "Node%sDegraded" - ) - - return func(nc *scyllav1alpha1.NodeConfig) (bool, error) { - if len(nodes) == 0 { - return true, fmt.Errorf("nodes can't be empty") - } - - for _, node := range nodes { - if nc.Status.ObservedGeneration < nc.Generation { - return false, nil - } - if !controllerhelpers.IsNodeTuned(nc.Status.NodeStatuses, node.Name) { - return false, nil - } - - availableNodeConditionType := scyllav1alpha1.NodeConfigConditionType(fmt.Sprintf(nodeAvailableConditionFormat, node.Name)) - progressingNodeConditionType := scyllav1alpha1.NodeConfigConditionType(fmt.Sprintf(nodeProgressingConditionFormat, node.Name)) - degradedNodeConditionType := scyllav1alpha1.NodeConfigConditionType(fmt.Sprintf(nodeDegradedConditionFormat, node.Name)) - - if !helpers.IsStatusNodeConfigConditionPresentAndTrue(nc.Status.Conditions, availableNodeConditionType, nc.Generation) { - return false, nil - } - if !helpers.IsStatusNodeConfigConditionPresentAndFalse(nc.Status.Conditions, progressingNodeConditionType, nc.Generation) { - return false, nil - } - if !helpers.IsStatusNodeConfigConditionPresentAndFalse(nc.Status.Conditions, degradedNodeConditionType, nc.Generation) { - return false, nil - } - } - - return true, nil - } -} - func IsNodeConfigDoneWithContainerTuningFunc(nodeName, containerID string) func(nc *scyllav1alpha1.NodeConfig) (bool, error) { return func(nc *scyllav1alpha1.NodeConfig) (bool, error) { containerTuned := controllerhelpers.IsNodeTunedForContainer(nc, nodeName, containerID) From 5a9d52c28798638c9b7f8c56a3936c0b50ef8079 Mon Sep 17 00:00:00 2001 From: Kacper Rzetelski Date: Thu, 29 Aug 2024 17:00:15 +0200 Subject: [PATCH 3/3] Update generated --- deploy/operator.yaml | 9 +++++++ .../scylla.scylladb.com_nodeconfigs.yaml | 9 +++++++ .../scylla/v1alpha1/zz_generated.deepcopy.go | 24 ++++++++++++++++++- 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/deploy/operator.yaml b/deploy/operator.yaml index b4ffeddac26..28e4550c852 100644 --- a/deploy/operator.yaml +++ b/deploy/operator.yaml @@ -311,6 +311,15 @@ spec: scope: Cluster versions: - additionalPrinterColumns: + - jsonPath: .status.conditions[?(@.type=='Available')].status + name: AVAILABLE + type: string + - jsonPath: .status.conditions[?(@.type=='Progressing')].status + name: PROGRESSING + type: string + - jsonPath: .status.conditions[?(@.type=='Degraded')].status + name: DEGRADED + type: string - jsonPath: .metadata.creationTimestamp name: AGE type: date diff --git a/pkg/api/scylla/v1alpha1/scylla.scylladb.com_nodeconfigs.yaml b/pkg/api/scylla/v1alpha1/scylla.scylladb.com_nodeconfigs.yaml index ca7f2b31b21..aa37cd46f4e 100644 --- a/pkg/api/scylla/v1alpha1/scylla.scylladb.com_nodeconfigs.yaml +++ b/pkg/api/scylla/v1alpha1/scylla.scylladb.com_nodeconfigs.yaml @@ -16,6 +16,15 @@ spec: scope: Cluster versions: - additionalPrinterColumns: + - jsonPath: .status.conditions[?(@.type=='Available')].status + name: AVAILABLE + type: string + - jsonPath: .status.conditions[?(@.type=='Progressing')].status + name: PROGRESSING + type: string + - jsonPath: .status.conditions[?(@.type=='Degraded')].status + name: DEGRADED + type: string - jsonPath: .metadata.creationTimestamp name: AGE type: date diff --git a/pkg/api/scylla/v1alpha1/zz_generated.deepcopy.go b/pkg/api/scylla/v1alpha1/zz_generated.deepcopy.go index 57c425a3040..4133c4367ed 100644 --- a/pkg/api/scylla/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/api/scylla/v1alpha1/zz_generated.deepcopy.go @@ -313,6 +313,28 @@ func (in *NodeConfigCondition) DeepCopy() *NodeConfigCondition { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in NodeConfigConditions) DeepCopyInto(out *NodeConfigConditions) { + { + in := &in + *out = make(NodeConfigConditions, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + return + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NodeConfigConditions. +func (in NodeConfigConditions) DeepCopy() NodeConfigConditions { + if in == nil { + return nil + } + out := new(NodeConfigConditions) + in.DeepCopyInto(out) + return *out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *NodeConfigList) DeepCopyInto(out *NodeConfigList) { *out = *in @@ -425,7 +447,7 @@ func (in *NodeConfigStatus) DeepCopyInto(out *NodeConfigStatus) { *out = *in if in.Conditions != nil { in, out := &in.Conditions, &out.Conditions - *out = make([]NodeConfigCondition, len(*in)) + *out = make(NodeConfigConditions, len(*in)) for i := range *in { (*in)[i].DeepCopyInto(&(*out)[i]) }