From a6c65753e431430e7dcad1186cbf207b343ddf67 Mon Sep 17 00:00:00 2001 From: Jiri Mencak Date: Thu, 14 Nov 2024 15:32:22 +0100 Subject: [PATCH] Manage TuneD profiles with the same name and different content It is possible to create Tuned CRs with TuneD profiles of the same name. This is problematic when the duplicate TuneD profiles have different contents. This can cause periodic switches between the duplicate TuneD profiles and will lead to TuneD restarts. This change makes the operator control loop stop to avoid switching between the conflicting TuneD profiles of the same name and warns about this misconfiguration via: * Tuned CR status and prometheus metric (nto_invalid_tuned_exist_info) * NTOInvalidTunedExist alert * ERROR message issued in the operator logs Other changes: * Switch the ProfileStatusCondition condition type to StatusCondition. This is cosmetic only, all fields are preserved. * Obsolete pod label-based matching. Trigger an alert (NTOPodLabelsUsed) for this. Fixes: OCPBUGS-44559. --- manifests/20-profile.crd.yaml | 2 +- manifests/20-tuned.crd.yaml | 44 ++++++++++- manifests/30-monitoring.yaml | 19 +++++ manifests/40-rbac.yaml | 2 +- pkg/apis/tuned/v1/tuned_types.go | 27 +++++-- pkg/apis/tuned/v1/zz_generated.deepcopy.go | 19 +++-- pkg/metrics/metrics.go | 18 +++++ pkg/operator/controller.go | 22 ++++-- pkg/operator/profilecalculator.go | 67 +++++++++++++--- pkg/operator/profilecalculator_test.go | 2 +- pkg/operator/status.go | 38 ++++++++- pkg/operator/validator.go | 79 +++++++++++++++++++ .../performanceprofile/status/status.go | 2 +- .../performanceprofile_controller_test.go | 2 +- pkg/tuned/controller.go | 2 +- pkg/tuned/status.go | 28 +++---- pkg/tuned/status_test.go | 18 ++--- test/e2e/deferred/operator_test.go | 6 +- .../functests/1_performance/performance.go | 2 +- test/e2e/util/util.go | 2 +- 20 files changed, 335 insertions(+), 66 deletions(-) create mode 100644 pkg/operator/validator.go diff --git a/manifests/20-profile.crd.yaml b/manifests/20-profile.crd.yaml index 57f26d39a8..9824bb4f8d 100644 --- a/manifests/20-profile.crd.yaml +++ b/manifests/20-profile.crd.yaml @@ -115,7 +115,7 @@ spec: description: conditions represents the state of the per-node Profile application type: array items: - description: ProfileStatusCondition represents a partial state of the per-node Profile application. + description: StatusCondition represents a partial state of the per-node Profile application. type: object required: - lastTransitionTime diff --git a/manifests/20-tuned.crd.yaml b/manifests/20-tuned.crd.yaml index 339a621ba8..d561afe0f0 100644 --- a/manifests/20-tuned.crd.yaml +++ b/manifests/20-tuned.crd.yaml @@ -18,7 +18,14 @@ spec: preserveUnknownFields: false scope: Namespaced versions: - - name: v1 + - additionalPrinterColumns: + - jsonPath: .status.conditions[?(@.type=="Valid")].status + name: Valid + type: string + - jsonPath: .metadata.creationTimestamp + name: Age + type: date + name: v1 schema: openAPIV3Schema: description: |- @@ -161,7 +168,42 @@ spec: type: object status: description: TunedStatus is the status for a Tuned resource. + properties: + conditions: + description: conditions represents the state of the Tuned profile + items: + description: StatusCondition represents a partial state of the per-node + Profile application. + properties: + lastTransitionTime: + description: lastTransitionTime is the time of the last update + to the current status property. + format: date-time + type: string + message: + description: |- + message provides additional information about the current condition. + This is only to be consumed by humans. + type: string + reason: + description: reason is the CamelCase reason for the condition's + current status. + type: string + status: + description: status of the condition, one of True, False, Unknown. + type: string + type: + description: type specifies the aspect reported by this condition. + type: string + required: + - lastTransitionTime + - status + - type + type: object + type: array type: object type: object served: true storage: true + subresources: + status: {} diff --git a/manifests/30-monitoring.yaml b/manifests/30-monitoring.yaml index 04ba071b9e..6dae51f04f 100644 --- a/manifests/30-monitoring.yaml +++ b/manifests/30-monitoring.yaml @@ -133,6 +133,25 @@ spec: for: 30m labels: severity: warning + - alert: NTOPodLabelsUsed + annotations: + description: >- + The Node Tuning Operator is using deprecated functionality. + Using pod label matching has been discouraged since OCP 4.4 and this functionality will be removed in future versions. + Please revise and adjust your configuration (Tuned custom resources). + summary: The Node Tuning Operator is using deprecated functionality. + expr: nto_pod_labels_used_info == 1 + for: 30m + labels: + severity: warning + - alert: NTOInvalidTunedExist + annotations: + description: Invalid custom Tuned resource exists. View your custom Tuned resources and operator logs for further details. + summary: Invalid custom Tuned resource exists. + expr: nto_invalid_tuned_exist_info == 1 + for: 30m + labels: + severity: warning - alert: NTODegraded annotations: description: The Node Tuning Operator is degraded. Review the "node-tuning" ClusterOperator object for further details. diff --git a/manifests/40-rbac.yaml b/manifests/40-rbac.yaml index 2a0826bdce..c83f0901d5 100644 --- a/manifests/40-rbac.yaml +++ b/manifests/40-rbac.yaml @@ -31,7 +31,7 @@ rules: resources: ["tuneds"] verbs: ["create","get","delete","list","update","watch","patch"] - apiGroups: ["tuned.openshift.io"] - resources: ["tuneds/finalizers"] + resources: ["tuneds/finalizers","tuneds/status"] verbs: ["update"] - apiGroups: ["tuned.openshift.io"] resources: ["profiles"] diff --git a/pkg/apis/tuned/v1/tuned_types.go b/pkg/apis/tuned/v1/tuned_types.go index f5b8a50d46..ca9070811e 100644 --- a/pkg/apis/tuned/v1/tuned_types.go +++ b/pkg/apis/tuned/v1/tuned_types.go @@ -34,6 +34,7 @@ const ( ///////////////////////////////////////////////////////////////////////////////// // +genclient // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object +// +kubebuilder:subresource:status // Tuned is a collection of rules that allows cluster-wide deployment // of node-level sysctls and more flexibility to add custom tuning @@ -134,8 +135,18 @@ type TuneDConfig struct { // TunedStatus is the status for a Tuned resource. type TunedStatus struct { + // conditions represents the state of the Tuned profile + // +patchMergeKey=type + // +patchStrategy=merge + // +optional + Conditions []StatusCondition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type"` } +const ( + // Tuned CR was validated and no problems with it were found. + TunedValid ConditionType = "Valid" +) + // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object // TunedList is a list of Tuned resources. @@ -192,16 +203,16 @@ type ProfileStatus struct { // +patchMergeKey=type // +patchStrategy=merge // +optional - Conditions []ProfileStatusCondition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type"` + Conditions []StatusCondition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type"` } -// ProfileStatusCondition represents a partial state of the per-node Profile application. +// StatusCondition represents a partial state of the per-node Profile application. // +k8s:deepcopy-gen=true -type ProfileStatusCondition struct { +type StatusCondition struct { // type specifies the aspect reported by this condition. // +kubebuilder:validation:Required // +required - Type ProfileConditionType `json:"type"` + Type ConditionType `json:"type"` // status of the condition, one of True, False, Unknown. // +kubebuilder:validation:Required @@ -223,18 +234,18 @@ type ProfileStatusCondition struct { Message string `json:"message,omitempty"` } -// ProfileConditionType is an aspect of Tuned daemon profile application state. -type ProfileConditionType string +// ConditionType is an aspect of Tuned daemon profile application state. +type ConditionType string const ( // ProfileApplied indicates that the Tuned daemon has successfully applied // the selected profile. - TunedProfileApplied ProfileConditionType = "Applied" + TunedProfileApplied ConditionType = "Applied" // TunedDegraded indicates the Tuned daemon issued errors during profile // application. To conclude the profile application was successful, // both TunedProfileApplied and TunedDegraded need to be queried. - TunedDegraded ProfileConditionType = "Degraded" + TunedDegraded ConditionType = "Degraded" ) // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object diff --git a/pkg/apis/tuned/v1/zz_generated.deepcopy.go b/pkg/apis/tuned/v1/zz_generated.deepcopy.go index 0f3954f707..900ec6ca59 100644 --- a/pkg/apis/tuned/v1/zz_generated.deepcopy.go +++ b/pkg/apis/tuned/v1/zz_generated.deepcopy.go @@ -148,7 +148,7 @@ func (in *ProfileStatus) DeepCopyInto(out *ProfileStatus) { *out = *in if in.Conditions != nil { in, out := &in.Conditions, &out.Conditions - *out = make([]ProfileStatusCondition, len(*in)) + *out = make([]StatusCondition, len(*in)) for i := range *in { (*in)[i].DeepCopyInto(&(*out)[i]) } @@ -167,18 +167,18 @@ func (in *ProfileStatus) DeepCopy() *ProfileStatus { } // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *ProfileStatusCondition) DeepCopyInto(out *ProfileStatusCondition) { +func (in *StatusCondition) DeepCopyInto(out *StatusCondition) { *out = *in in.LastTransitionTime.DeepCopyInto(&out.LastTransitionTime) return } -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ProfileStatusCondition. -func (in *ProfileStatusCondition) DeepCopy() *ProfileStatusCondition { +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new StatusCondition. +func (in *StatusCondition) DeepCopy() *StatusCondition { if in == nil { return nil } - out := new(ProfileStatusCondition) + out := new(StatusCondition) in.DeepCopyInto(out) return out } @@ -210,7 +210,7 @@ func (in *Tuned) DeepCopyInto(out *Tuned) { out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) in.Spec.DeepCopyInto(&out.Spec) - out.Status = in.Status + in.Status.DeepCopyInto(&out.Status) return } @@ -403,6 +403,13 @@ func (in *TunedSpec) DeepCopy() *TunedSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *TunedStatus) DeepCopyInto(out *TunedStatus) { *out = *in + if in.Conditions != nil { + in, out := &in.Conditions, &out.Conditions + *out = make([]StatusCondition, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } return } diff --git a/pkg/metrics/metrics.go b/pkg/metrics/metrics.go index f549a8dc7a..375222ab3a 100644 --- a/pkg/metrics/metrics.go +++ b/pkg/metrics/metrics.go @@ -8,6 +8,7 @@ const ( profileCalculatedQuery = "nto_profile_calculated_total" buildInfoQuery = "nto_build_info" degradedInfoQuery = "nto_degraded_info" + invalidTunedExistQuery = "nto_invalid_tuned_exist_info" // MetricsPort is the IP port supplied to the HTTP server used for Prometheus, // and matches what is specified in the corresponding Service and ServiceMonitor. @@ -42,6 +43,12 @@ var ( Help: "Indicates whether the Node Tuning Operator is degraded.", }, ) + invalidTunedExist = prometheus.NewGauge( + prometheus.GaugeOpts{ + Name: invalidTunedExistQuery, + Help: "Does any invalid/misconfigured custom Tuned resource exist (1) or not (0)?", + }, + ) ) func init() { @@ -50,6 +57,7 @@ func init() { profileCalculated, buildInfo, degradedState, + invalidTunedExist, ) } @@ -83,3 +91,13 @@ func Degraded(deg bool) { } degradedState.Set(0) } + +// InvalidTunedExist indicates whether any invalid/misconfigured custom Tuned resource +// exist or not. +func InvalidTunedExist(enable bool) { + if enable { + invalidTunedExist.Set(1) + return + } + invalidTunedExist.Set(0) +} diff --git a/pkg/operator/controller.go b/pkg/operator/controller.go index 9d47ce12b1..536bedbc6f 100644 --- a/pkg/operator/controller.go +++ b/pkg/operator/controller.go @@ -417,6 +417,11 @@ func (c *Controller) sync(key wqKey) error { // Tuned CR changed, this can affect all profiles, list them and trigger profile updates klog.V(2).Infof("sync(): Tuned %s", key.name) + err = c.validateTunedCRs() + if err != nil { + return err + } + err = c.enqueueProfileUpdates() if err != nil { return err @@ -500,6 +505,7 @@ func (c *Controller) syncTunedDefault() (*tunedv1.Tuned, error) { if err != nil { return cr, fmt.Errorf("failed to update Tuned %s: %v", crMf.Name, err) } + return cr, nil } @@ -613,14 +619,18 @@ func (c *Controller) syncProfile(tuned *tunedv1.Tuned, nodeName string) error { var computed ComputedProfile if ntoconfig.InHyperShift() { computed, err = c.pc.calculateProfileHyperShift(nodeName) - if err != nil { - return err - } } else { computed, err = c.pc.calculateProfile(nodeName) - if err != nil { - return err - } + } + switch err.(type) { + case nil: + case *DuplicateProfileError: + // Stop. We have TuneD profiles with the same name and different contents. + // Do not spam the logs with this error, it will be reported during Tuned CR + // updates and periodic resync/validation. + return nil + default: + return err } metrics.ProfileCalculated(profileMf.Name, computed.TunedProfileName) diff --git a/pkg/operator/profilecalculator.go b/pkg/operator/profilecalculator.go index 185aa6d908..a7310b4e52 100644 --- a/pkg/operator/profilecalculator.go +++ b/pkg/operator/profilecalculator.go @@ -182,7 +182,11 @@ func (pc *ProfileCalculator) calculateProfile(nodeName string) (ComputedProfile, return ComputedProfile{}, fmt.Errorf("failed to list Tuned: %v", err) } - profilesAll := tunedProfiles(tunedList) + profilesAll, err := tunedProfiles(tunedList) + if err != nil { + return ComputedProfile{}, err + } + recommendAll := TunedRecommend(tunedList) recommendProfile := func(nodeName string, iStart int) (int, RecommendedProfile, error) { var i int @@ -345,7 +349,11 @@ func (pc *ProfileCalculator) calculateProfileHyperShift(nodeName string) (Comput } tunedList = append(tunedList, defaultTuned) - profilesAll := tunedProfiles(tunedList) + profilesAll, err := tunedProfiles(tunedList) + if err != nil { + return ComputedProfile{}, err + } + recommendAll := TunedRecommend(tunedList) recommendProfile := func(nodeName string, iStart int) (int, HypershiftRecommendedProfile, error) { var i int @@ -675,6 +683,16 @@ func (pc *ProfileCalculator) tunedsUseNodeLabels(tunedSlice []*tunedv1.Tuned) bo return false } +// tunedMatchesPodLabels returns true if Tuned CRs 'tuned' uses Pod labels. +func (pc *ProfileCalculator) tunedMatchesPodLabels(tuned *tunedv1.Tuned) bool { + for _, recommend := range tuned.Spec.Recommend { + if pc.tunedUsesPodLabels(recommend.Match) { + return true + } + } + return false +} + // tunedsUsePodLabels returns true if any of the Tuned CRs uses Pod labels. func (pc *ProfileCalculator) tunedsUsePodLabels(tunedSlice []*tunedv1.Tuned) bool { for _, recommend := range TunedRecommend(tunedSlice) { @@ -692,10 +710,12 @@ func (pc *ProfileCalculator) getNodePoolNameForNode(node *corev1.Node) (string, return nodePoolName, nil } -// tunedProfiles returns a name-sorted TunedProfile slice out of -// a slice of Tuned objects. -func tunedProfiles(tunedSlice []*tunedv1.Tuned) []tunedv1.TunedProfile { - tunedProfiles := []tunedv1.TunedProfile{} +// tunedProfilesGet returns a TunedProfile map out of Tuned objects. +// Returns an error when TuneD profiles with the same name and different +// contents are detected. +func tunedProfilesGet(tunedSlice []*tunedv1.Tuned) (map[string]tunedv1.TunedProfile, error) { + var dups bool // Do we have any duplicate TuneD profiles with different content? + tuned2crName := map[string][]string{} m := map[string]tunedv1.TunedProfile{} for _, tuned := range tunedSlice { @@ -708,14 +728,43 @@ func tunedProfiles(tunedSlice []*tunedv1.Tuned) []tunedv1.TunedProfile { } if existingProfile, found := m[*v.Name]; found { if *v.Data == *existingProfile.Data { - klog.Infof("duplicate profiles names %s but they have the same contents", *v.Name) + klog.Infof("duplicate profiles %s but they have the same contents", *v.Name) } else { - klog.Errorf("ERROR: duplicate profiles named %s with different contents found in Tuned CR %q", *v.Name, tuned.Name) + // Duplicate profiles "*v.Name" with different content detected in Tuned CR "tuned.Name" + // Do not spam the logs with this error, it will be logged elsewhere. + dups = true } } m[*v.Name] = v + tuned2crName[*v.Name] = append(tuned2crName[*v.Name], tuned.Name) + } + } + + if dups { + duplicates := map[string]string{} + + for k, v := range tuned2crName { + if len(v) < 2 { + continue + } + for _, vv := range v { + duplicates[vv] = k // Tuned CR name (vv) -> duplicate TuneD profile (k) mapping; ignore multiple duplicates to keep things simple + } } + + return m, &DuplicateProfileError{crName: duplicates} } + + return m, nil +} + +// tunedProfiles returns a name-sorted TunedProfile slice out of a slice of +// Tuned objects. Returns an error when TuneD profiles with the same name and +// different contents are detected. +func tunedProfiles(tunedSlice []*tunedv1.Tuned) ([]tunedv1.TunedProfile, error) { + tunedProfiles := []tunedv1.TunedProfile{} + m, err := tunedProfilesGet(tunedSlice) + for _, tunedProfile := range m { tunedProfiles = append(tunedProfiles, tunedProfile) } @@ -727,7 +776,7 @@ func tunedProfiles(tunedSlice []*tunedv1.Tuned) []tunedv1.TunedProfile { return *tunedProfiles[i].Name < *tunedProfiles[j].Name }) - return tunedProfiles + return tunedProfiles, err } type TunedRecommendInfo struct { diff --git a/pkg/operator/profilecalculator_test.go b/pkg/operator/profilecalculator_test.go index baa26e9014..cf5e437ced 100644 --- a/pkg/operator/profilecalculator_test.go +++ b/pkg/operator/profilecalculator_test.go @@ -123,7 +123,7 @@ func TestTunedProfiles(t *testing.T) { ) for i, tc := range tests { - tunedProfilesSorted := tunedProfiles(tc.input) + tunedProfilesSorted, _ := tunedProfiles(tc.input) if !reflect.DeepEqual(tc.expectedOutput, tunedProfilesSorted) { t.Errorf( diff --git a/pkg/operator/status.go b/pkg/operator/status.go index 682533c35a..5d9fec4169 100644 --- a/pkg/operator/status.go +++ b/pkg/operator/status.go @@ -19,6 +19,7 @@ import ( ntoconfig "github.com/openshift/cluster-node-tuning-operator/pkg/config" ntomf "github.com/openshift/cluster-node-tuning-operator/pkg/manifests" "github.com/openshift/cluster-node-tuning-operator/pkg/metrics" + "github.com/openshift/cluster-node-tuning-operator/pkg/tuned" ) const ( @@ -74,7 +75,7 @@ func (c *Controller) syncOperatorStatus(tuned *tunedv1.Tuned) error { return nil } -// getOrCreateOperatorStatus get the currect ClusterOperator object or creates a new one. +// getOrCreateOperatorStatus gets the currect ClusterOperator object or creates a new one. // The method always returns a DeepCopy() of the object so the return value is safe to use // for Update()s. func (c *Controller) getOrCreateOperatorStatus() (*configv1.ClusterOperator, error) { @@ -111,7 +112,7 @@ func profileApplied(profile *tunedv1.Profile) bool { } // profileDegraded returns true if Profile 'profile' is Degraded. -// The Degraded ProfileStatusCondition occurs when a TuneD reports errors applying +// The Degraded StatusCondition occurs when a TuneD reports errors applying // the profile or when there is a timeout waiting for the profile to be applied. func profileDegraded(profile *tunedv1.Profile) bool { if profile == nil { @@ -386,3 +387,36 @@ func conditionTrue(conditions []configv1.ClusterOperatorStatusCondition, condTyp return false } + +// initializeTunedStatusConditions returns a slice of tunedv1.StatusCondition +// initialized to an unknown state. +func initializeTunedStatusConditions() []tunedv1.StatusCondition { + now := metav1.Now() + return []tunedv1.StatusCondition{ + { + Type: tunedv1.TunedValid, + Status: corev1.ConditionUnknown, + LastTransitionTime: now, + }, + } +} + +// computeTunedStatus computes the Tuned CR's status. +func computeStatusConditions(tunedValid bool, message string, conditions []tunedv1.StatusCondition) []tunedv1.StatusCondition { + tunedValidCondition := tunedv1.StatusCondition{ + Type: tunedv1.TunedValid, + } + + if tunedValid { + tunedValidCondition.Status = corev1.ConditionTrue + tunedValidCondition.Reason = "AsExpected" + } else { + tunedValidCondition.Status = corev1.ConditionFalse + tunedValidCondition.Reason = "InvalidConfiguration" + } + tunedValidCondition.Message = message + + conditions = tuned.SetStatusCondition(conditions, &tunedValidCondition) + + return conditions +} diff --git a/pkg/operator/validator.go b/pkg/operator/validator.go new file mode 100644 index 0000000000..d124f4ba0d --- /dev/null +++ b/pkg/operator/validator.go @@ -0,0 +1,79 @@ +package operator + +import ( + "context" + "fmt" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/klog/v2" + + ntoconfig "github.com/openshift/cluster-node-tuning-operator/pkg/config" + "github.com/openshift/cluster-node-tuning-operator/pkg/metrics" +) + +type DuplicateProfileError struct { + crName map[string]string + // ^^^^^^ TuneD profile duplicate name (record only one duplicate to keep things simple). + // ^^^^^^ Tuned CR name containing a duplicate TuneD profile. +} + +func (e *DuplicateProfileError) Error() string { + return fmt.Sprintf("ERROR: duplicate TuneD profiles found: %v", e.crName) +} + +// validateTunedCRs checks all Tuned CR for potential misconfiguration, +// obsolete functionality and updates their status as necessary. +// Returns error only if either List()/Update() of/on k8s objects needs +// to be requeued. +func (c *Controller) validateTunedCRs() error { + allTunedValid := true + + tunedList, err := c.listers.TunedResources.List(labels.Everything()) + if err != nil { + return fmt.Errorf("failed to list Tuned: %v", err) + } + + _, errProfilesGet := tunedProfilesGet(tunedList) + + for _, tuned := range tunedList { + tunedValid := true + message := "" // Do not add any unnecessary clutter when configuration is valid. + + // Check for deprecated functionality first. + if c.pc.tunedMatchesPodLabels(tuned) { + tunedValid = false + allTunedValid = false + klog.Errorf("tuned/%s uses pod label matching", tuned.Name) + message = "Deprecated pod label matching detected." + } + + switch e := errProfilesGet.(type) { + case nil: + case *DuplicateProfileError: + // We have TuneD profiles with the same name and different contents. + if val, found := e.crName[tuned.Name]; found { + tunedValid = false + allTunedValid = false + klog.Errorf("duplicate TuneD profile %s with conflicting content detected in tuned/%s.", val, tuned.Name) + message = fmt.Sprintf("Duplicate TuneD profile %q with conflicting content detected.", val) + } + } + + tuned = tuned.DeepCopy() // Make sure we do not modify objects in cache + + if len(tuned.Status.Conditions) == 0 { + tuned.Status.Conditions = initializeTunedStatusConditions() + } + tuned.Status.Conditions = computeStatusConditions(tunedValid, message, tuned.Status.Conditions) + + _, err = c.clients.Tuned.TunedV1().Tuneds(ntoconfig.WatchNamespace()).UpdateStatus(context.TODO(), tuned, metav1.UpdateOptions{}) + if err != nil { + return fmt.Errorf("failed to update Tuned %s status: %v", tuned.Name, err) + } + } + // Populate NTO metrics to create an alert if the misconfiguration persists. + metrics.InvalidTunedExist(!allTunedValid) + + return nil +} diff --git a/pkg/performanceprofile/controller/performanceprofile/status/status.go b/pkg/performanceprofile/controller/performanceprofile/status/status.go index f2b77fb7f8..e532a422ef 100644 --- a/pkg/performanceprofile/controller/performanceprofile/status/status.go +++ b/pkg/performanceprofile/controller/performanceprofile/status/status.go @@ -209,7 +209,7 @@ func GetTunedProfilesMessage(profiles []tunedv1.Profile) string { for _, tunedProfile := range profiles { isDegraded := false isApplied := true - var tunedDegradedCondition *tunedv1.ProfileStatusCondition + var tunedDegradedCondition *tunedv1.StatusCondition for i := 0; i < len(tunedProfile.Status.Conditions); i++ { condition := &tunedProfile.Status.Conditions[i] diff --git a/pkg/performanceprofile/controller/performanceprofile_controller_test.go b/pkg/performanceprofile/controller/performanceprofile_controller_test.go index 77cf062a5c..9c6fab8af0 100644 --- a/pkg/performanceprofile/controller/performanceprofile_controller_test.go +++ b/pkg/performanceprofile/controller/performanceprofile_controller_test.go @@ -696,7 +696,7 @@ var _ = Describe("Controller", func() { Name: "tuned-profile-test", }, Status: tunedv1.ProfileStatus{ - Conditions: []tunedv1.ProfileStatusCondition{ + Conditions: []tunedv1.StatusCondition{ { Type: tunedv1.TunedDegraded, Status: corev1.ConditionTrue, diff --git a/pkg/tuned/controller.go b/pkg/tuned/controller.go index 6f218d734a..f372550841 100644 --- a/pkg/tuned/controller.go +++ b/pkg/tuned/controller.go @@ -1357,7 +1357,7 @@ func (c *Controller) updateTunedProfileStatus(ctx context.Context, change Change c.daemon.status = daemonStatus if profile.Status.TunedProfile == activeProfile && - conditionsEqual(profile.Status.Conditions, statusConditions) { + ConditionsEqual(profile.Status.Conditions, statusConditions) { klog.V(2).Infof("updateTunedProfileStatus(): no need to update status of Profile %s", profile.Name) return nil } diff --git a/pkg/tuned/status.go b/pkg/tuned/status.go index 64013ec9e1..0c9d17fa20 100644 --- a/pkg/tuned/status.go +++ b/pkg/tuned/status.go @@ -7,12 +7,12 @@ import ( tunedv1 "github.com/openshift/cluster-node-tuning-operator/pkg/apis/tuned/v1" ) -// setStatusCondition returns the result of setting the specified condition in +// SetStatusCondition returns the result of setting the specified condition in // the given slice of conditions. -func setStatusCondition(oldConditions []tunedv1.ProfileStatusCondition, condition *tunedv1.ProfileStatusCondition) []tunedv1.ProfileStatusCondition { +func SetStatusCondition(oldConditions []tunedv1.StatusCondition, condition *tunedv1.StatusCondition) []tunedv1.StatusCondition { condition.LastTransitionTime = metav1.Now() - newConditions := []tunedv1.ProfileStatusCondition{} + newConditions := []tunedv1.StatusCondition{} found := false for _, c := range oldConditions { @@ -36,9 +36,9 @@ func setStatusCondition(oldConditions []tunedv1.ProfileStatusCondition, conditio return newConditions } -// conditionsEqual returns true if and only if the provided slices of conditions +// ConditionsEqual returns true if and only if the provided slices of conditions // (ignoring LastTransitionTime) are equal. -func conditionsEqual(oldConditions, newConditions []tunedv1.ProfileStatusCondition) bool { +func ConditionsEqual(oldConditions, newConditions []tunedv1.StatusCondition) bool { if len(newConditions) != len(oldConditions) { return false } @@ -65,11 +65,11 @@ func conditionsEqual(oldConditions, newConditions []tunedv1.ProfileStatusConditi return true } -// InitializeStatusConditions returns a slice of tunedv1.ProfileStatusCondition +// InitializeStatusConditions returns a slice of tunedv1.StatusCondition // initialized to an unknown state. -func InitializeStatusConditions() []tunedv1.ProfileStatusCondition { +func InitializeStatusConditions() []tunedv1.StatusCondition { now := metav1.Now() - return []tunedv1.ProfileStatusCondition{ + return []tunedv1.StatusCondition{ { Type: tunedv1.TunedProfileApplied, Status: corev1.ConditionUnknown, @@ -85,19 +85,19 @@ func InitializeStatusConditions() []tunedv1.ProfileStatusCondition { // computeStatusConditions takes the set of Bits 'status', old conditions // 'conditions', an optional 'message' to put in the relevant condition field, -// and returns an updated slice of tunedv1.ProfileStatusCondition. +// and returns an updated slice of tunedv1.StatusCondition. // 'status' contains all the information necessary for creating a new slice of // conditions apart from LastTransitionTime, which is set based on checking the // old conditions. -func computeStatusConditions(status Bits, message string, conditions []tunedv1.ProfileStatusCondition) []tunedv1.ProfileStatusCondition { +func computeStatusConditions(status Bits, message string, conditions []tunedv1.StatusCondition) []tunedv1.StatusCondition { if (status & scUnknown) != 0 { return InitializeStatusConditions() } - tunedProfileAppliedCondition := tunedv1.ProfileStatusCondition{ + tunedProfileAppliedCondition := tunedv1.StatusCondition{ Type: tunedv1.TunedProfileApplied, } - tunedDegradedCondition := tunedv1.ProfileStatusCondition{ + tunedDegradedCondition := tunedv1.StatusCondition{ Type: tunedv1.TunedDegraded, } @@ -142,8 +142,8 @@ func computeStatusConditions(status Bits, message string, conditions []tunedv1.P tunedDegradedCondition.Message = "No warning or error messages observed applying the TuneD daemon profile." } - conditions = setStatusCondition(conditions, &tunedProfileAppliedCondition) - conditions = setStatusCondition(conditions, &tunedDegradedCondition) + conditions = SetStatusCondition(conditions, &tunedProfileAppliedCondition) + conditions = SetStatusCondition(conditions, &tunedDegradedCondition) return conditions } diff --git a/pkg/tuned/status_test.go b/pkg/tuned/status_test.go index b592272a42..ffbd0eeb63 100644 --- a/pkg/tuned/status_test.go +++ b/pkg/tuned/status_test.go @@ -16,12 +16,12 @@ func TestComputeStatusConditions(t *testing.T) { name string status Bits stderr string - conds []tunedv1.ProfileStatusCondition - expected []tunedv1.ProfileStatusCondition + conds []tunedv1.StatusCondition + expected []tunedv1.StatusCondition }{ { name: "nil", - expected: []tunedv1.ProfileStatusCondition{ + expected: []tunedv1.StatusCondition{ { Type: tunedv1.TunedProfileApplied, Status: corev1.ConditionFalse, @@ -45,7 +45,7 @@ func TestComputeStatusConditions(t *testing.T) { { name: "only-deferred", status: scDeferred, - expected: []tunedv1.ProfileStatusCondition{ + expected: []tunedv1.StatusCondition{ { Type: tunedv1.TunedProfileApplied, Status: corev1.ConditionFalse, @@ -70,7 +70,7 @@ func TestComputeStatusConditions(t *testing.T) { name: "error-deferred", status: scError | scDeferred, stderr: "test-error", - expected: []tunedv1.ProfileStatusCondition{ + expected: []tunedv1.StatusCondition{ { Type: tunedv1.TunedProfileApplied, Status: corev1.ConditionFalse, @@ -95,7 +95,7 @@ func TestComputeStatusConditions(t *testing.T) { name: "sysctl-deferred", status: scSysctlOverride | scDeferred, stderr: "test-error", - expected: []tunedv1.ProfileStatusCondition{ + expected: []tunedv1.StatusCondition{ { Type: tunedv1.TunedProfileApplied, Status: corev1.ConditionFalse, @@ -120,7 +120,7 @@ func TestComputeStatusConditions(t *testing.T) { name: "warning-deferred", status: scWarn | scDeferred, stderr: "test-error", - expected: []tunedv1.ProfileStatusCondition{ + expected: []tunedv1.StatusCondition{ { Type: tunedv1.TunedProfileApplied, Status: corev1.ConditionFalse, @@ -152,8 +152,8 @@ func TestComputeStatusConditions(t *testing.T) { } } -func clearTimestamps(conds []tunedv1.ProfileStatusCondition) []tunedv1.ProfileStatusCondition { - ret := make([]tunedv1.ProfileStatusCondition, 0, len(conds)) +func clearTimestamps(conds []tunedv1.StatusCondition) []tunedv1.StatusCondition { + ret := make([]tunedv1.StatusCondition, 0, len(conds)) for idx := range conds { cond := conds[idx] // local copy cond.LastTransitionTime = metav1.Time{ diff --git a/test/e2e/deferred/operator_test.go b/test/e2e/deferred/operator_test.go index dc6d3ef22f..5dbbe4f76f 100644 --- a/test/e2e/deferred/operator_test.go +++ b/test/e2e/deferred/operator_test.go @@ -138,7 +138,7 @@ func setDeferred(obj *tunedv1.Tuned, mode ntoutil.DeferMode) *tunedv1.Tuned { return obj } -func findCondition(conditions []tunedv1.ProfileStatusCondition, conditionType tunedv1.ProfileConditionType) *tunedv1.ProfileStatusCondition { +func findCondition(conditions []tunedv1.StatusCondition, conditionType tunedv1.ConditionType) *tunedv1.StatusCondition { for _, condition := range conditions { if condition.Type == conditionType { return &condition @@ -147,7 +147,7 @@ func findCondition(conditions []tunedv1.ProfileStatusCondition, conditionType tu return nil } -func checkAppliedConditionDeferred(cond *tunedv1.ProfileStatusCondition, expectedProfile string) error { +func checkAppliedConditionDeferred(cond *tunedv1.StatusCondition, expectedProfile string) error { klog.Infof("expected profile: %q", expectedProfile) if cond.Status != corev1.ConditionFalse { return fmt.Errorf("applied is true") @@ -158,7 +158,7 @@ func checkAppliedConditionDeferred(cond *tunedv1.ProfileStatusCondition, expecte return nil } -func checkAppliedConditionOK(cond *tunedv1.ProfileStatusCondition) error { +func checkAppliedConditionOK(cond *tunedv1.StatusCondition) error { if cond.Status != corev1.ConditionTrue { return fmt.Errorf("applied is false") } diff --git a/test/e2e/performanceprofile/functests/1_performance/performance.go b/test/e2e/performanceprofile/functests/1_performance/performance.go index d9fb35dc77..154b79aa71 100644 --- a/test/e2e/performanceprofile/functests/1_performance/performance.go +++ b/test/e2e/performanceprofile/functests/1_performance/performance.go @@ -1424,7 +1424,7 @@ func makeDevRPSMap(content string) map[string]string { } // Helper function to find a condition in the status.conditions slice by type -func findCondition(conditions []tunedv1.ProfileStatusCondition, conditionType string) *tunedv1.ProfileStatusCondition { +func findCondition(conditions []tunedv1.StatusCondition, conditionType string) *tunedv1.StatusCondition { for _, condition := range conditions { if string(condition.Type) == conditionType { return &condition diff --git a/test/e2e/util/util.go b/test/e2e/util/util.go index 36e85d18d8..98f74b035e 100644 --- a/test/e2e/util/util.go +++ b/test/e2e/util/util.go @@ -349,7 +349,7 @@ func WaitForClusterOperatorConditionReason(cs *framework.ClientSet, interval, du // The execution interval to check the value is 'interval' and retries last // for at most the duration 'duration'. func WaitForProfileConditionStatus(cs *framework.ClientSet, interval, duration time.Duration, profile string, profileExpect string, - conditionType tunedv1.ProfileConditionType, conditionStatus corev1.ConditionStatus) error { + conditionType tunedv1.ConditionType, conditionStatus corev1.ConditionStatus) error { var explain error startTime := time.Now()