Skip to content

Commit

Permalink
controller: update MC after application by TuneD (#352)
Browse files Browse the repository at this point in the history
Tuned Profiles are created by NTO operator and updated by both the
operator and its operand.  The operand's updates are the source of truth
for information such as kernel command-line parameters and presence of
the stalld daemon.  The operator is informed about this via the status
field of the a Tuned Profile that corresponds to a given node.  Operator
updates are the source of truth about which TuneD profile should be
applied.

When a new cluster node is added (for example during a cluster
scale-up), a new Tuned Profile is created.  If the node is quickly added
to a MachinePool for which an NTO-managed MachineConfig already exists,
this addition will cause NTO to overwrite/update the MachineConfig with
data that is incorrectly initialized.  If the operand was too slow to
correct this update (typically during scale-ups when the operand was not
yet running), this would cause unnecessary reboots of nodes sharing the
same MachineConfigPool.

This PR fixes the behaviour above by letting the operator update the
NTO-managed MachineConfigs only once the newly requested Tuned Profile
configuration is successfully applied.

Other changes:
- go mod vendor && go mod tidy

Resolves rhbz#2024682

Co-authored-by: Jiri Mencak <jmencak@users.noreply.github.com>
  • Loading branch information
jmencak and jmencak committed Apr 26, 2022
1 parent 9316487 commit ddbc574
Show file tree
Hide file tree
Showing 8 changed files with 263 additions and 13 deletions.
37 changes: 37 additions & 0 deletions manifests/20-crd-profile.yaml
Expand Up @@ -58,11 +58,48 @@ spec:
is for internal use only and its fields may be changed/removed in the
future.
type: object
required:
- tunedProfile
properties:
bootcmdline:
description: kernel parameters calculated by tuned for the active
Tuned profile
type: string
conditions:
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.
type: object
required:
- lastTransitionTime
- status
- type
properties:
lastTransitionTime:
description: lastTransitionTime is the time of the last update
to the current status property.
type: string
format: date-time
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
stalld:
description: 'deploy stall daemon: https://github.com/bristot/stalld/'
type: boolean
tunedProfile:
description: the current profile in use by the Tuned daemon
type: string
56 changes: 55 additions & 1 deletion pkg/apis/tuned/v1/tuned_types.go
@@ -1,8 +1,10 @@
package v1

import (
operatorv1 "github.com/openshift/api/operator/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

operatorv1 "github.com/openshift/api/operator/v1"
)

const (
Expand Down Expand Up @@ -144,11 +146,63 @@ type ProfileStatus struct {
// kernel parameters calculated by tuned for the active Tuned profile
// +optional
Bootcmdline string `json:"bootcmdline"`

// the current profile in use by the Tuned daemon
TunedProfile string `json:"tunedProfile"`

// conditions represents the state of the per-node Profile application
// +patchMergeKey=type
// +patchStrategy=merge
// +optional
Conditions []ProfileStatusCondition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type"`

// deploy stall daemon: https://github.com/bristot/stalld/
// +optional
Stalld *bool `json:"stalld"`
}

// ProfileStatusCondition represents a partial state of the per-node Profile application.
// +k8s:deepcopy-gen=true
type ProfileStatusCondition struct {
// type specifies the aspect reported by this condition.
// +kubebuilder:validation:Required
// +required
Type ProfileConditionType `json:"type"`

// status of the condition, one of True, False, Unknown.
// +kubebuilder:validation:Required
// +required
Status corev1.ConditionStatus `json:"status"`

// lastTransitionTime is the time of the last update to the current status property.
// +kubebuilder:validation:Required
// +required
LastTransitionTime metav1.Time `json:"lastTransitionTime"`

// reason is the CamelCase reason for the condition's current status.
// +optional
Reason string `json:"reason,omitempty"`

// message provides additional information about the current condition.
// This is only to be consumed by humans.
// +optional
Message string `json:"message,omitempty"`
}

// ProfileConditionType is an aspect of Tuned daemon profile application state.
type ProfileConditionType string

const (
// ProfileApplied indicates that the Tuned daemon has successfully applied
// the selected profile.
TunedProfileApplied ProfileConditionType = "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"
)

// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
// ProfileList is a list of Profile resources
type ProfileList struct {
Expand Down
24 changes: 24 additions & 0 deletions pkg/apis/tuned/v1/zz_generated.deepcopy.go

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

12 changes: 8 additions & 4 deletions pkg/operator/controller.go
Expand Up @@ -586,12 +586,16 @@ func (c *Controller) syncProfile(tuned *tunedv1.Tuned, nodeName string) error {
}

if mcLabels != nil {
// The tuned profile "tunedProfileName" for nodeName matched with MachineConfig
// The Tuned daemon profile 'tunedProfileName' for nodeName matched with MachineConfig
// labels set for additional machine configuration. Sync the operator-created
// MachineConfig for MachineConfigPools 'pools'.
err := c.syncMachineConfig(getMachineConfigNameForPools(pools), mcLabels, profile.Status.Bootcmdline, profile.Status.Stalld)
if err != nil {
return fmt.Errorf("failed to update Profile %s: %v", profile.Name, err)
if profile.Status.TunedProfile == tunedProfileName && profileApplied(profile) {
// Synchronize MachineConfig only once the (calculated) TuneD profile 'tunedProfileName'
// has been successfully applied.
err := c.syncMachineConfig(getMachineConfigNameForPools(pools), mcLabels, profile.Status.Bootcmdline, profile.Status.Stalld)
if err != nil {
return fmt.Errorf("failed to update Profile %s: %v", profile.Name, err)
}
}
}
if profile.Spec.Config.TunedProfile == tunedProfileName &&
Expand Down
16 changes: 16 additions & 0 deletions pkg/operator/status.go
Expand Up @@ -8,6 +8,7 @@ import (
configv1 "github.com/openshift/api/config/v1"
operatorv1 "github.com/openshift/api/operator/v1"
operatorv1helpers "github.com/openshift/library-go/pkg/operator/v1helpers"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/klog/v2"
Expand Down Expand Up @@ -89,6 +90,21 @@ func (c *Controller) getOrCreateOperatorStatus() (*configv1.ClusterOperator, err
return co, nil
}

// profileApplied returns true if Tuned Profile 'profile' has been applied.
func profileApplied(profile *tunedv1.Profile) bool {
if profile == nil || profile.Spec.Config.TunedProfile != profile.Status.TunedProfile {
return false
}

for _, sc := range profile.Status.Conditions {
if sc.Type == tunedv1.TunedProfileApplied && sc.Status == corev1.ConditionTrue {
return true
}
}

return false
}

// computeStatusConditions computes the operator's current state.
func (c *Controller) computeStatusConditions(tuned *tunedv1.Tuned, conditions []configv1.ClusterOperatorStatusCondition,
dsName string) ([]configv1.ClusterOperatorStatusCondition, error) {
Expand Down
109 changes: 109 additions & 0 deletions pkg/tuned/status.go
@@ -0,0 +1,109 @@
package tuned

import (
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

tunedv1 "github.com/openshift/cluster-node-tuning-operator/pkg/apis/tuned/v1"
)

// 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 {
condition.LastTransitionTime = metav1.Now()

newConditions := []tunedv1.ProfileStatusCondition{}

found := false
for _, c := range oldConditions {
if condition.Type == c.Type {
if condition.Status == c.Status &&
condition.Reason == c.Reason &&
condition.Message == c.Message {
return oldConditions
}

found = true
newConditions = append(newConditions, *condition)
} else {
newConditions = append(newConditions, c)
}
}
if !found {
newConditions = append(newConditions, *condition)
}

return newConditions
}

// conditionsEqual returns true if and only if the provided slices of conditions
// (ignoring LastTransitionTime) are equal.
func conditionsEqual(oldConditions, newConditions []tunedv1.ProfileStatusCondition) bool {
if len(newConditions) != len(oldConditions) {
return false
}

for _, conditionA := range oldConditions {
foundMatchingCondition := false

for _, conditionB := range newConditions {
// Compare every field except LastTransitionTime.
if conditionA.Type == conditionB.Type &&
conditionA.Status == conditionB.Status &&
conditionA.Reason == conditionB.Reason &&
conditionA.Message == conditionB.Message {
foundMatchingCondition = true
break
}
}

if !foundMatchingCondition {
return false
}
}

return true
}

// computeStatusConditions takes the set of Bits 'status', old conditions
// 'conditions' and returns an updated slice of tunedv1.ProfileStatusCondition.
// '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, conditions []tunedv1.ProfileStatusCondition) []tunedv1.ProfileStatusCondition {
tunedProfileAppliedCondition := tunedv1.ProfileStatusCondition{
Type: tunedv1.TunedProfileApplied,
}
tunedDegradedCondition := tunedv1.ProfileStatusCondition{
Type: tunedv1.TunedDegraded,
}

if (status & scApplied) != 0 {
tunedProfileAppliedCondition.Status = corev1.ConditionTrue
tunedProfileAppliedCondition.Reason = "AsExpected"
tunedProfileAppliedCondition.Message = "Tuned profile applied."
} else {
tunedProfileAppliedCondition.Status = corev1.ConditionFalse
tunedProfileAppliedCondition.Reason = "Failed"
tunedProfileAppliedCondition.Message = "The Tuned daemon profile application failed."
}

if (status & scError) != 0 {
tunedDegradedCondition.Status = corev1.ConditionTrue
tunedDegradedCondition.Reason = "TunedError"
tunedDegradedCondition.Message = "Tuned daemon issued one or more error message(s) during profile application."
} else if (status & scWarn) != 0 {
tunedDegradedCondition.Status = corev1.ConditionFalse // consider warnings from Tuned as non-fatal
tunedDegradedCondition.Reason = "TunedWarning"
tunedDegradedCondition.Message = "No error messages observed by applying the Tuned daemon profile, only warning(s)."
} else {
tunedDegradedCondition.Status = corev1.ConditionFalse
tunedDegradedCondition.Reason = "AsExpected"
tunedDegradedCondition.Message = "No warning or error messages observed applying the Tuned daemon profile."
}

conditions = setStatusCondition(conditions, &tunedProfileAppliedCondition)
conditions = setStatusCondition(conditions, &tunedDegradedCondition)

return conditions
}
16 changes: 14 additions & 2 deletions pkg/tuned/tuned.go
Expand Up @@ -825,10 +825,20 @@ func (c *Controller) updateTunedProfile() (err error) {
return fmt.Errorf("unable to assess whether stalld is requested: %v", err)
}

activeProfile, err := getActiveProfile()
if err != nil {
return err
}

statusConditions := computeStatusConditions(c.daemon.status, profile.Status.Conditions)

stalldUnchanged := util.PtrBoolEqual(profile.Status.Stalld, stalldRequested)

if profile.Status.Bootcmdline == bootcmdline && stalldUnchanged {
// Do not update node profile unnecessarily (e.g. bootcmdline did not change)
if profile.Status.Bootcmdline == bootcmdline && stalldUnchanged &&
profile.Status.TunedProfile == activeProfile && conditionsEqual(profile.Status.Conditions, statusConditions) {
// Do not update node Profile unnecessarily (e.g. bootcmdline did not change).
// This will save operator CPU cycles trying to reconcile objects that do not
// need reconciling.
klog.V(2).Infof("updateTunedProfile(): no need to update Profile %s", profile.Name)
return nil
}
Expand All @@ -837,6 +847,8 @@ func (c *Controller) updateTunedProfile() (err error) {

profile.Status.Bootcmdline = bootcmdline
profile.Status.Stalld = stalldRequested
profile.Status.TunedProfile = activeProfile
profile.Status.Conditions = statusConditions
_, err = c.clients.Tuned.TunedV1().Profiles(operandNamespace).Update(context.TODO(), profile, metav1.UpdateOptions{})
if err != nil {
return fmt.Errorf("failed to update Profile %s status: %v", profile.Name, err)
Expand Down
6 changes: 0 additions & 6 deletions vendor/modules.txt
Expand Up @@ -12,8 +12,6 @@ github.com/cespare/xxhash/v2
github.com/coreos/go-semver/semver
# github.com/coreos/go-systemd/v22 v22.0.0
github.com/coreos/go-systemd/v22/unit
# github.com/coreos/ignition v0.35.0
## explicit
# github.com/coreos/ignition/v2 v2.7.0
## explicit
github.com/coreos/ignition/v2/config/merge
Expand Down Expand Up @@ -157,8 +155,6 @@ github.com/openshift/client-go/config/informers/externalversions/config
github.com/openshift/client-go/config/informers/externalversions/config/v1
github.com/openshift/client-go/config/informers/externalversions/internalinterfaces
github.com/openshift/client-go/config/listers/config/v1
# github.com/openshift/crd-schema-gen v1.0.0
## explicit
# github.com/openshift/library-go v0.0.0-20210113192829-cfbb3f4c80c2
## explicit
github.com/openshift/library-go/pkg/operator/v1helpers
Expand Down Expand Up @@ -198,8 +194,6 @@ github.com/spf13/pflag
# github.com/vincent-petithory/dataurl v0.0.0-20191104211930-d1553a71de50
## explicit
github.com/vincent-petithory/dataurl
# go4.org v0.0.0-20201209231011-d4a079459e60
## explicit
# golang.org/x/crypto v0.0.0-20201002170205-7f63de1d35b0
golang.org/x/crypto/ssh/terminal
# golang.org/x/mod v0.3.0
Expand Down

0 comments on commit ddbc574

Please sign in to comment.