Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

OCPBUGS-32469: Remove tuned/rendered object #1036

Merged
merged 5 commits into from
Apr 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions manifests/20-profile.crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,25 @@ spec:
required:
- tunedProfile
type: object
profile:
description: Tuned profiles.
items:
description: A Tuned profile.
properties:
data:
description: Specification of the Tuned profile to be consumed
by the Tuned daemon.
type: string
name:
description: Name of the Tuned profile to be used in the recommend
section.
minLength: 1
type: string
required:
- data
- name
type: object
type: array
required:
- config
type: object
Expand Down
3 changes: 3 additions & 0 deletions pkg/apis/tuned/v1/tuned_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,9 @@ type Profile struct {

type ProfileSpec struct {
Config ProfileConfig `json:"config"`
// Tuned profiles.
// +optional
Profile []TunedProfile `json:"profile"`
}

type ProfileConfig struct {
Expand Down
7 changes: 7 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.

14 changes: 12 additions & 2 deletions pkg/generated/informers/externalversions/factory.go

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

61 changes: 0 additions & 61 deletions pkg/manifests/manifests.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,12 @@ import (
"bytes"
"io"
"os"
"sort"

appsv1 "k8s.io/api/apps/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/yaml"

tunedv1 "github.com/openshift/cluster-node-tuning-operator/pkg/apis/tuned/v1"
ntoconfig "github.com/openshift/cluster-node-tuning-operator/pkg/config"
"k8s.io/klog/v2"
)

const (
Expand All @@ -26,47 +23,6 @@ func MustAssetReader(asset string) io.Reader {
return bytes.NewReader(MustAsset(asset))
}

func TunedRenderedResource(tunedSlice []*tunedv1.Tuned) *tunedv1.Tuned {
cr := &tunedv1.Tuned{
ObjectMeta: metav1.ObjectMeta{
Name: tunedv1.TunedRenderedResourceName,
Namespace: ntoconfig.WatchNamespace(),
},
Spec: tunedv1.TunedSpec{
Recommend: []tunedv1.TunedRecommend{},
},
}

tunedProfiles := []tunedv1.TunedProfile{}
m := map[string]tunedv1.TunedProfile{}

for _, tuned := range tunedSlice {
if tuned.Name == tunedv1.TunedRenderedResourceName {
// Skip the "rendered" Tuned resource itself
continue
}
tunedRenderedProfiles(tuned, m)
}
for _, tunedProfile := range m {
if tunedProfile.Name == nil {
// This should never happen (openAPIV3Schema validation); ignore invalid profiles
continue
}
tunedProfiles = append(tunedProfiles, tunedProfile)
}

// The order of Tuned resources is variable and so is the order of profiles
// within the resource itself. Sort the rendered profiles by their names for
// simpler change detection.
sort.Slice(tunedProfiles, func(i, j int) bool {
return *tunedProfiles[i].Name < *tunedProfiles[j].Name
})

cr.Spec.Profile = tunedProfiles

return cr
}

func TunedDaemonSet() *appsv1.DaemonSet {
ds, err := NewDaemonSet(MustAssetReader(TunedDaemonSetAsset))
if err != nil {
Expand Down Expand Up @@ -126,20 +82,3 @@ func NewTuned(manifest io.Reader) (*tunedv1.Tuned, error) {
}
return &o, nil
}

func tunedRenderedProfiles(tuned *tunedv1.Tuned, m map[string]tunedv1.TunedProfile) {
if tuned.Spec.Profile != nil {
for _, v := range tuned.Spec.Profile {
if v.Name != nil && v.Data != nil {
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)
} else {
klog.Errorf("ERROR: duplicate profiles named %s with different contents found in Tuned CR %q", *v.Name, tuned.Name)
}
}
m[*v.Name] = v
}
}
}
}
105 changes: 45 additions & 60 deletions pkg/operator/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -397,10 +397,11 @@ func (c *Controller) sync(key wqKey) error {
}
}

klog.V(2).Infof("sync(): Tuned %s", tunedv1.TunedRenderedResourceName)
err = c.syncTunedRendered(cr)
klog.V(2).Infof("sync(): enableInformers")
// Enable/disable node/pod informers based on existing TuneD CRs.
err = c.enableInformers()
if err != nil {
return fmt.Errorf("failed to sync Tuned %s: %v", tunedv1.TunedRenderedResourceName, err)
return fmt.Errorf("failed to enable/disable informers: %v", err)
}

if key.name != tunedv1.TunedDefaultResourceName {
Expand Down Expand Up @@ -428,11 +429,6 @@ func (c *Controller) sync(key wqKey) error {
return err
}

if key.name == tunedv1.TunedRenderedResourceName {
// Do not start unused MachineConfig pruning unnecessarily for the rendered resource
return nil
}

// Tuned CR change can also mean some MachineConfigs the operator created are no longer needed;
// removal of these will also rollback host settings such as kernel boot parameters.
if ntoconfig.InHyperShift() {
Expand Down Expand Up @@ -514,16 +510,12 @@ func (c *Controller) syncTunedDefault() (*tunedv1.Tuned, error) {
return cr, nil
}

func (c *Controller) syncTunedRendered(tuned *tunedv1.Tuned) error {
func (c *Controller) enableInformers() error {
tunedList, err := c.listers.TunedResources.List(labels.Everything())
if err != nil {
return fmt.Errorf("failed to list Tuned: %v", err)
}

crMf := ntomf.TunedRenderedResource(tunedList)
crMf.ObjectMeta.OwnerReferences = getDefaultTunedRefs(tuned)
crMf.Name = tunedv1.TunedRenderedResourceName

nodeLabelsUsed := c.pc.tunedsUseNodeLabels(tunedList)
if err = c.enableNodeInformer(nodeLabelsUsed); err != nil {
return fmt.Errorf("failed to enable Node informer: %v", err)
Expand All @@ -536,35 +528,6 @@ func (c *Controller) syncTunedRendered(tuned *tunedv1.Tuned) error {
return fmt.Errorf("failed to enable Pod informer: %v", err)
}

cr, err := c.listers.TunedResources.Get(tunedv1.TunedRenderedResourceName)
if err != nil {
if errors.IsNotFound(err) {
klog.V(2).Infof("syncTunedRendered(): Tuned %s not found, creating one", crMf.Name)
_, err = c.clients.Tuned.TunedV1().Tuneds(ntoconfig.WatchNamespace()).Create(context.TODO(), crMf, metav1.CreateOptions{})
if err != nil {
return fmt.Errorf("failed to create Tuned %s: %v", crMf.Name, err)
}
// Tuned created successfully
klog.Infof("created Tuned %s", crMf.Name)
return nil
}
return fmt.Errorf("failed to get Tuned %s: %v", tunedv1.TunedRenderedResourceName, err)
}

if reflect.DeepEqual(crMf.Spec.Profile, cr.Spec.Profile) {
klog.V(2).Infof("syncTunedRendered(): Tuned %s doesn't need updating", crMf.Name)
return nil
}
cr = cr.DeepCopy() // never update the objects from cache
cr.Spec = crMf.Spec

klog.V(2).Infof("syncTunedRendered(): updating Tuned %s", cr.Name)
_, err = c.clients.Tuned.TunedV1().Tuneds(ntoconfig.WatchNamespace()).Update(context.TODO(), cr, metav1.UpdateOptions{})
if err != nil {
return fmt.Errorf("failed to update Tuned %s: %v", cr.Name, err)
}
klog.Infof("updated Tuned %s", cr.Name)

return nil
}

Expand Down Expand Up @@ -627,6 +590,7 @@ func (c *Controller) syncDaemonSet(tuned *tunedv1.Tuned) error {
func (c *Controller) syncProfile(tuned *tunedv1.Tuned, nodeName string) error {
var (
tunedProfileName string
profilesAll []tunedv1.TunedProfile
mcLabels map[string]string
operand tunedv1.OperandConfig
nodePoolName string
Expand Down Expand Up @@ -661,23 +625,19 @@ func (c *Controller) syncProfile(tuned *tunedv1.Tuned, nodeName string) error {
}

if ntoconfig.InHyperShift() {
tunedProfileName, nodePoolName, operand, err = c.pc.calculateProfileHyperShift(nodeName)
tunedProfileName, profilesAll, nodePoolName, operand, err = c.pc.calculateProfileHyperShift(nodeName)
if err != nil {
return err
}
} else {
tunedProfileName, mcLabels, operand, err = c.pc.calculateProfile(nodeName)
tunedProfileName, profilesAll, mcLabels, operand, err = c.pc.calculateProfile(nodeName)
if err != nil {
return err
}
}

metrics.ProfileCalculated(profileMf.Name, tunedProfileName)

if err != nil {
return fmt.Errorf("failed to get Tuned %s: %v", tunedv1.TunedRenderedResourceName, err)
}

profile, err := c.listers.TunedProfiles.Get(profileMf.Name)
if err != nil {
if errors.IsNotFound(err) {
Expand All @@ -694,6 +654,7 @@ func (c *Controller) syncProfile(tuned *tunedv1.Tuned, nodeName string) error {
profileMf.Spec.Config.TunedProfile = tunedProfileName
profileMf.Spec.Config.Debug = operand.Debug
profileMf.Spec.Config.TuneDConfig = operand.TuneDConfig
profileMf.Spec.Profile = profilesAll
profileMf.Status.Conditions = tunedpkg.InitializeStatusConditions()
_, err = c.clients.Tuned.TunedV1().Profiles(ntoconfig.WatchNamespace()).Create(context.TODO(), profileMf, metav1.CreateOptions{})
if err != nil {
Expand Down Expand Up @@ -754,6 +715,7 @@ func (c *Controller) syncProfile(tuned *tunedv1.Tuned, nodeName string) error {
if profile.Spec.Config.TunedProfile == tunedProfileName &&
profile.Spec.Config.Debug == operand.Debug &&
reflect.DeepEqual(profile.Spec.Config.TuneDConfig, operand.TuneDConfig) &&
reflect.DeepEqual(profile.Spec.Profile, profilesAll) &&
profile.Spec.Config.ProviderName == providerName {
klog.V(2).Infof("syncProfile(): no need to update Profile %s", nodeName)
return nil
Expand All @@ -762,6 +724,7 @@ func (c *Controller) syncProfile(tuned *tunedv1.Tuned, nodeName string) error {
profile.Spec.Config.TunedProfile = tunedProfileName
profile.Spec.Config.Debug = operand.Debug
profile.Spec.Config.TuneDConfig = operand.TuneDConfig
profile.Spec.Profile = profilesAll
profile.Spec.Config.ProviderName = providerName
profile.Status.Conditions = tunedpkg.InitializeStatusConditions()

Expand Down Expand Up @@ -1274,6 +1237,30 @@ func (c *Controller) enablePodInformer(enable bool) error {
return nil
}

// Remove this function and associated code in the future. This is for cleanup during upgrades only.
// The rendered resource is no longer used.
func (c *Controller) removeTunedRendered() error {
var err error

_, err = c.listers.TunedResources.Get(tunedv1.TunedRenderedResourceName)
if err != nil {
if errors.IsNotFound(err) {
// Do not create any noise when TunedRenderedResourceName is not found (was already removed).
err = nil
} else {
err = fmt.Errorf("failed to get Tuned %s: %v", tunedv1.TunedRenderedResourceName, err)
}
} else {
err = c.clients.Tuned.TunedV1().Tuneds(ntoconfig.WatchNamespace()).Delete(context.TODO(), tunedv1.TunedRenderedResourceName, metav1.DeleteOptions{})
if err != nil {
err = fmt.Errorf("failed to delete Tuned %s: %v", tunedv1.TunedRenderedResourceName, err)
} else {
klog.Infof("deleted Tuned %s", tunedv1.TunedRenderedResourceName)
}
}
return err
}

func (c *Controller) removeResources() error {
var lastErr error
dsMf := ntomf.TunedDaemonSet()
Expand All @@ -1293,18 +1280,10 @@ func (c *Controller) removeResources() error {
}
}

_, err = c.listers.TunedResources.Get(tunedv1.TunedRenderedResourceName)
if err != nil {
if !errors.IsNotFound(err) {
lastErr = fmt.Errorf("failed to get Tuned %s: %v", tunedv1.TunedRenderedResourceName, err)
}
} else {
err = c.clients.Tuned.TunedV1().Tuneds(ntoconfig.WatchNamespace()).Delete(ctx, tunedv1.TunedRenderedResourceName, metav1.DeleteOptions{})
if err != nil {
lastErr = fmt.Errorf("failed to delete Tuned %s: %v", tunedv1.TunedRenderedResourceName, err)
} else {
klog.Infof("deleted Tuned %s", tunedv1.TunedRenderedResourceName)
}
// Remove this code in the future. This is for cleanup during upgrades only.
// The rendered resource is no longer used.
if err := c.removeTunedRendered(); err != nil {
lastErr = err
}

profileList, err := c.listers.TunedProfiles.List(labels.Everything())
Expand Down Expand Up @@ -1451,6 +1430,12 @@ func (c *Controller) run(ctx context.Context) error {
return fmt.Errorf("failed to wait for caches to sync")
}

// Remove this code in the future. This is for cleanup during upgrades only.
// The rendered resource is no longer used.
if err := c.removeTunedRendered(); err != nil {
klog.Error(err)
}

klog.V(1).Info("starting events processor")
go wait.Until(c.eventProcessor, time.Second, ctx.Done())
klog.Info("started events processor/controller")
Expand Down
2 changes: 1 addition & 1 deletion pkg/operator/hypershift.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ func (c *Controller) syncHostedClusterTuneds() error {
}
// Anything left in hcMap should be deleted
for tunedName := range hcTunedMap {
if tunedName != tunedv1.TunedDefaultResourceName && tunedName != tunedv1.TunedRenderedResourceName {
if tunedName != tunedv1.TunedDefaultResourceName {
klog.V(1).Infof("deleting stale Tuned %s in hosted cluster", tunedName)
err = c.clients.Tuned.TunedV1().Tuneds(ntoconfig.WatchNamespace()).Delete(context.TODO(), tunedName, metav1.DeleteOptions{})
if err != nil {
Expand Down
Loading