From df66093eeb2be9fa5a35a67a268a8dc26f4ecf2a Mon Sep 17 00:00:00 2001 From: Francesco Romani Date: Mon, 22 Jul 2024 16:38:15 +0200 Subject: [PATCH] tuned: controller: pack profilesExtract return values in struct xref: https://github.com/openshift/cluster-node-tuning-operator/pull/1019#discussion_r1686119969 Signed-off-by: Francesco Romani --- pkg/tuned/cmd/render/render.go | 2 +- pkg/tuned/controller.go | 58 +++++++++++++++++++++++----------- 2 files changed, 41 insertions(+), 19 deletions(-) diff --git a/pkg/tuned/cmd/render/render.go b/pkg/tuned/cmd/render/render.go index 7c54073e95..7765da20c7 100644 --- a/pkg/tuned/cmd/render/render.go +++ b/pkg/tuned/cmd/render/render.go @@ -213,7 +213,7 @@ func render(inputDir []string, outputDir string, mcpName string) error { for _, t := range tuneD { tunedProfiles = append(tunedProfiles, t.Spec.Profile...) } - _, _, _, _, err = tunedpkg.ProfilesExtract(tunedProfiles, recommendedProfile) + _, err = tunedpkg.ProfilesExtract(tunedProfiles, recommendedProfile) if err != nil { klog.Errorf("error extracting tuned profiles : %v", err) return fmt.Errorf("error extracting tuned profiles: %w", err) diff --git a/pkg/tuned/controller.go b/pkg/tuned/controller.go index 40cb2b420b..783b5e5b3a 100644 --- a/pkg/tuned/controller.go +++ b/pkg/tuned/controller.go @@ -389,15 +389,23 @@ func profilesEqual(profileFile string, profileData string) bool { return profileData == string(content) } +type ExtractedProfiles struct { + // True if the data in the to-be-extracted recommended profile or the profiles being + // included from the current recommended profile have changed. + Changed bool + // If the data changed, the fingerprint of the new profile, or "" otherwise. + Fingerprint string + // A map with successfully extracted TuneD profile names. + Names map[string]bool + // A map with names of TuneD profiles the current TuneD recommended profile depends on. + Dependencies map[string]bool +} + // ProfilesExtract extracts TuneD daemon profiles to tunedProfilesDirCustom directory. // Returns: -// - True if the data in the to-be-extracted recommended profile or the profiles being -// included from the current recommended profile have changed. -// - If the data changed, the fingerprint of the new profile, or "" otherwise. -// - A map with successfully extracted TuneD profile names. -// - A map with names of TuneD profiles the current TuneD recommended profile depends on. +// - ExtractedProfile with the details of the operation performed // - Error if any or nil. -func ProfilesExtract(profiles []tunedv1.TunedProfile, recommendedProfile string) (bool, string, map[string]bool, map[string]bool, error) { +func ProfilesExtract(profiles []tunedv1.TunedProfile, recommendedProfile string) (ExtractedProfiles, error) { klog.Infof("profilesExtract(): extracting %d TuneD profiles (recommended=%s)", len(profiles), recommendedProfile) // Get a list of TuneD profiles names the recommended profile depends on. deps := profileDepends(recommendedProfile) @@ -409,7 +417,7 @@ func ProfilesExtract(profiles []tunedv1.TunedProfile, recommendedProfile string) // profilesExtractPathWithDeps is like ProfilesExtract but takes explicit profiles root dir path and // explicit dependencies, so it's easier to test. To be used only internally. -func profilesExtractPathWithDeps(profilesRootDir string, profiles []tunedv1.TunedProfile, recommendedProfile string, recommendedProfileDeps map[string]bool) (bool, string, map[string]bool, map[string]bool, error) { +func profilesExtractPathWithDeps(profilesRootDir string, profiles []tunedv1.TunedProfile, recommendedProfile string, recommendedProfileDeps map[string]bool) (ExtractedProfiles, error) { var ( change bool = false extracted map[string]bool = map[string]bool{} // TuneD profile names present in TuneD CR and successfully extracted to tunedProfilesDirCustom @@ -428,7 +436,11 @@ func profilesExtractPathWithDeps(profilesRootDir string, profiles []tunedv1.Tune profileFile := filepath.Join(profileDir, tunedConfFile) if err := os.MkdirAll(profileDir, os.ModePerm); err != nil { - return change, "", extracted, recommendedProfileDeps, fmt.Errorf("failed to create TuneD profile directory %q: %v", profileDir, err) + return ExtractedProfiles{ + Changed: change, + Names: extracted, + Dependencies: recommendedProfileDeps, + }, fmt.Errorf("failed to create TuneD profile directory %q: %v", profileDir, err) } if recommendedProfileDeps[*profile.Name] { @@ -444,7 +456,11 @@ func profilesExtractPathWithDeps(profilesRootDir string, profiles []tunedv1.Tune err := os.WriteFile(profileFile, []byte(*profile.Data), 0644) if err != nil { - return change, "", extracted, recommendedProfileDeps, fmt.Errorf("failed to write TuneD profile file %q: %v", profileFile, err) + return ExtractedProfiles{ + Changed: change, + Names: extracted, + Dependencies: recommendedProfileDeps, + }, fmt.Errorf("failed to write TuneD profile file %q: %v", profileFile, err) } extracted[*profile.Name] = true klog.V(2).Infof("profilesExtract(): extracted profile %q to %q (%d bytes)", *profile.Name, profileFile, len(*profile.Data)) @@ -452,7 +468,12 @@ func profilesExtractPathWithDeps(profilesRootDir string, profiles []tunedv1.Tune profilesFP := profilesFingerprint(profiles, recommendedProfile) klog.Infof("profilesExtract(): fingerprint of extracted profiles: %q", profilesFP) - return change, profilesFP, extracted, recommendedProfileDeps, nil + return ExtractedProfiles{ + Changed: change, + Fingerprint: profilesFP, + Names: extracted, + Dependencies: recommendedProfileDeps, + }, nil } // profilesRepackPath reconstructs the TunedProfile object from the data unpacked on the node @@ -506,16 +527,17 @@ func profilesRepackPath(recommendFilePath, profilesRootDir string) ([]tunedv1.Tu // - The synced profile fingerprint. // - Error if any or nil. func profilesSync(profiles []tunedv1.TunedProfile, recommendedProfile string) (bool, string, error) { - change, profilesFP, extractedNew, recommendedProfileDeps, err := ProfilesExtract(profiles, recommendedProfile) + extracted, err := ProfilesExtract(profiles, recommendedProfile) if err != nil { - return change, "", err + return extracted.Changed, "", err } dirEntries, err := os.ReadDir(tunedProfilesDirCustom) if err != nil { - return change, "", err + return extracted.Changed, "", err } + changed := extracted.Changed // shortcut, but we also modify before // Deal with TuneD profiles absent from Tuned CRs, but still present in // for _, dirEntry := range dirEntries { profile := dirEntry.Name() @@ -529,7 +551,7 @@ func profilesSync(profiles []tunedv1.TunedProfile, recommendedProfile string) (b continue } - if extractedNew[profile] { + if extracted.Names[profile] { // Do not delete the freshly extracted profiles. These are the profiles present in the Profile CR. continue } @@ -537,18 +559,18 @@ func profilesSync(profiles []tunedv1.TunedProfile, recommendedProfile string) (b profileDir := fmt.Sprintf("%s/%s", tunedProfilesDirCustom, profile) err := os.RemoveAll(profileDir) if err != nil { - return change, "", fmt.Errorf("failed to remove %q: %v", profileDir, err) + return changed, "", fmt.Errorf("failed to remove %q: %v", profileDir, err) } klog.Infof("profilesSync(): removed TuneD profile %q", profileDir) - if recommendedProfileDeps[profile] { + if extracted.Dependencies[profile] { // This TuneD profile does not exist in the Profile CR, but the recommended profile depends on it. // Trigger a change to report a configuration issue -- we depend on a profile that does not exist. - change = true + changed = true } } - return change, profilesFP, nil + return changed, extracted.Fingerprint, nil } // filterAndSortProfiles returns a slice of valid (non-nil name, non-nil data) profiles