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.

Sign up for GitHub

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

Jump to bottom

Aggregate NodeConfig status conditions #1918

Merged
merged 3 commits into from
Sep 6, 2024
Merged

Aggregate NodeConfig status conditions #1918

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0564dba
Aggregate NodeConfig status conditions and add server-side printers
rzetelskik Aug 29, 2024
8b71353
Accomodate NodeConfig condition aggregation in e2e tests and add a te…
rzetelskik Aug 29, 2024
5a9d52c
Update generated
rzetelskik Aug 29, 2024
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
9 changes: 9 additions & 0 deletions deploy/operator.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions pkg/api/scylla/v1alpha1/scylla.scylladb.com_nodeconfigs.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions pkg/api/scylla/v1alpha1/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// Copyright (C) 2024 ScyllaDB

package v1alpha1

const (
AvailableCondition = "Available"
ProgressingCondition = "Progressing"
DegradedCondition = "Degraded"
)
6 changes: 0 additions & 6 deletions pkg/api/scylla/v1alpha1/types_monitoring.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
47 changes: 45 additions & 2 deletions pkg/api/scylla/v1alpha1/types_nodeconfig.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 {
tnozicka marked this conversation as resolved.
Show resolved Hide resolved
// type is the type of the NodeConfig condition.
Type NodeConfigConditionType `json:"type"`
Expand All @@ -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"`
Expand Down Expand Up @@ -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 {
Expand Down
24 changes: 23 additions & 1 deletion pkg/api/scylla/v1alpha1/zz_generated.deepcopy.go
View file
Open in desktop

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

20 changes: 20 additions & 0 deletions pkg/controller/nodeconfig/conditions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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"
)
50 changes: 7 additions & 43 deletions pkg/controller/nodeconfig/status.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,72 +4,36 @@ 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,
}
Comment on lines -37 to -40
Copy link
Collaborator

Choose a reason for hiding this comment

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

Where it is being set now? I don't see it in PR.

Copy link
Member Author

@rzetelskik rzetelskik Aug 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

it isn't - progressing and available conditions are used instead

Copy link
Collaborator

Choose a reason for hiding this comment

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

then your should raise the apiversion to v1alpha2

Copy link
Member

Choose a reason for hiding this comment

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

conditions are not technically part of the API version / can be unknown or unset - but I agree, it should be kept, at least for a while, to retain compatibility and behaviour

Copy link
Member

Choose a reason for hiding this comment

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

I'd put this into syncDaemonSet though, something like

	desiredSum := int64(0)
	allReconciled := true
	for _, requiredDaemonSet := range requiredDaemonSets {
		if requiredDaemonSet == nil {
			continue
		}

		ds, _, err := resourceapply.ApplyDaemonSet(ctx, ncc.kubeClient.AppsV1(), ncc.daemonSetLister, ncc.eventRecorder, requiredDaemonSet, resourceapply.ApplyOptions{})
		if err != nil {
			return progressingConditions, fmt.Errorf("can't apply daemonset: %w", err)
		}

		desiredSum += int64(ds.Status.DesiredNumberScheduled)

		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)
		}
		if !reconciled {
			allReconciled = false
		}
	}

	status.DesiredNodeSetupCount = pointer.Ptr(desiredSum)

	reconciledCondition := metav1.Condition{
		Type:               string(scyllav1alpha1.NodeConfigReconciledConditionType),
		ObservedGeneration: nc.Generation,
		Status:             metav1.ConditionUnknown,
	}

	if allReconciled {
		reconciledCondition.Status = metav1.ConditionTrue
		reconciledCondition.Reason = "FullyReconciledAndUp"
		reconciledCondition.Message = "All operands are reconciled and available."
	} else {
		reconciledCondition.Status = metav1.ConditionFalse
		reconciledCondition.Reason = "DaemonSetNotRolledOut"
		reconciledCondition.Message = "DaemonSet isn't reconciled and fully rolled out yet."
	}
	_ = apimeta.SetStatusCondition(statusConditions, reconciledCondition)

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll add it back, although the tests won't be depending on it

Copy link
Member Author

Choose a reason for hiding this comment

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

done

Copy link
Member Author

Choose a reason for hiding this comment

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

@tnozicka is there any benefit in doing this over what I did with computing this from available/progressing/degraded conditions?

Copy link
Member

Choose a reason for hiding this comment

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

I haven't seen the new projection at a time I wrote this - looking at it, it changes the semantics of it but I guess that's ok


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 {
if apiequality.Semantic.DeepEqual(&currentNodeConfig.Status, status) {
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
}
Loading