[release-4.17] OCPBUGS-38721: tuned: distinguish deferred updates #1149
Conversation
* OCPBUGS-28647: tuned: distinguish deferred updates To fully support the usecase described in OCPBUGS-28647 and fix the issue, we need to further distinguish between first-time profile change and in-place profile change. This is required to better support a GitOps flow. The key distinction is if the recommended profile changes or not, and there's a desire to defer application of changes only when a profile is updated (e.g. sysctl modified), not the first time it is applied. Thus: - first-time profile change is a change which triggers a change of the recommended profile. - in-place profile update is a change which does NOT cause a switch to a TuneD profile with a different name. This involves changes to only the contents of the currently used profile. We change the way the annotation is used. We now require a value, which can be either - always: every Tuned object annotated this way will have its application deferred. - update: every Tuned object annotated this way will be processed as usual (and as it wasn't annotated) if it's a first-time profile change, but its in-place updates will be deferred. - a new internal value "never" is also added to be used internally to mean the deferred feature is disabled entirely. User can use this value but it will explicitly disable the feature (which is disabled already by default), thus is redundant and not recommended. Signed-off-by: Francesco Romani <[email protected]> * e2e: drop tags, use labels now that we have the more powerful ginkgo labels, we can stop using tags for newer tests. Signed-off-by: Francesco Romani <[email protected]> --------- Signed-off-by: Francesco Romani <[email protected]>
…enshift#1142) * OCPBUGS-38795: Fix defer status during recommended profile change Process recommended profile change even when the profile itself has not changed itself. The internal profile fingerprint tracking was not updated during recommended profile update with no internal changes. * Add e2e test for defer=update
openshift-ci-robot
added
jira/valid-reference
|
@yanirq: This pull request references Jira Issue OCPBUGS-38721, which is valid. 7 validation(s) were run on this bug
Requesting review from QA contact: The bug has been updated to refer to the pull request using the external bug tracker. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: yanirq The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
openshift-ci
bot
requested review from
ffromani,
jmencak and
MarSik
and removed request for
kpouget
…enshift#1162) * log: make sure to have high verbosity in tests Signed-off-by: Francesco Romani <[email protected]> * e2e: deferred: stricter testing add stricter check for the node condition when validating OCPBUGS-38795 Signed-off-by: Francesco Romani <[email protected]> * deferred: tuned: fix reload trigger when inplace update There are conditions on which we should not set the reload flag. This avoid regression in the test "Profile deferred when applied should trigger changes when applied fist, then deferred when edited, if tuned restart should be kept deferred" Signed-off-by: Francesco Romani <[email protected]> * deferred: tuned: clarify comments and logs document better the logic about processing edits Signed-off-by: Francesco Romani <[email protected]> --------- Signed-off-by: Francesco Romani <[email protected]>
|
/retest |
pkg/tuned/controller.go
Outdated
| err = util.SetLogLevel(profile.Spec.Config.Verbosity) | ||
| if err != nil { | ||
| klog.Errorf("failed to set log level %d: %v", profile.Spec.Config.Verbosity, err) | ||
| } else { | ||
| klog.Infof("set log level %d", profile.Spec.Config.Verbosity) | ||
| } |
There was a problem hiding this comment.
this hunk depends on a unbackported PR: #1114 . We shold either also backport it (probably not the best approach) or remove this hunk in a separate commit in this PR.
There was a problem hiding this comment.
Honestly, I tend to lean towards the latter. Dropping this hunk.
|
@yanirq: all tests passed! Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
What makes 4.17.0 special vs. 4.17.z? You need backport-risk-assessed for 4.17.z too and customers are more likely wait for 4.17.z than running with 4.17.0 immediately. |
Yes you are right. scratch the version reference. |
|
/lgtm |
|
/label backport-risk-assessed |
openshift-ci
bot
added
the
backport-risk-assessed
|
we need |
|
@liqcui , can we please have cherry-pick-approved label for this? Thank you! |
|
/label cherry-pick-approved |
openshift-ci
bot
added
the
cherry-pick-approved
openshift-merge-bot
bot
merged commit c77c557
into
openshift:release-4.17
|
@yanirq: Jira Issue OCPBUGS-38721: All pull requests linked via external trackers have merged: Jira Issue OCPBUGS-38721 has been moved to the MODIFIED state. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
[ART PR BUILD NOTIFIER] Distgit: cluster-node-tuning-operator |
This is a manual cherry pick of:
#1129 OCPBUGS-28647: tuned: distinguish deferred updates
#1142 OCPBUGS-38795: Fix defer status during recommended profile change
#1162 NO-JIRA: deferred updates: fix in-place update handling on reboot