CNF-11145: Enhance render sync to include bootstrap rendering tests #932
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: MarSik 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
| for f in "${WORKDIR}"/test/e2e/performanceprofile/testdata/render-expected-output/${OUTPUT_DIR}/* | ||
| do | ||
| sed -i "s/uid:.*/uid: \"\"/" "${f}" | ||
| done |
There was a problem hiding this comment.
After extra thinking, this should probably be done once to reconcile the current state with the expected state, and then not enforced at every sync.
Other than this LGTM
There was a problem hiding this comment.
Extra: I'd remove the entry entirely rather than set to empty.
There was a problem hiding this comment.
I actually did not change this part, just moved it into a function.
But the sync is the reconciliation of current state with expected state. The uid reset only happend on make render-sync to make sure the tests themselves do not have to do that anymore.
IOW: I do not understand what do you mean by "once" here.
|
/retest-required This PR only affects the e2e-no-cluster lane directly, but lets see if the CI is healthier than yesterday. |
MarSik
force-pushed
the
master-render-sync-fix
branch
from
415b5c8
to
a494c18
Compare
|
@MarSik: 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/test-infra repository. I understand the commands that are listed here. |
MarSik
changed the title
|
@MarSik: This pull request references CNF-11145 which is a valid jira issue. 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. |
openshift-ci-robot
added
the
jira/valid-reference
|
/jira refresh |
|
@MarSik: This pull request references CNF-11145 which is a valid jira issue. 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. |
|
/lgtm |
|
[ART PR BUILD NOTIFIER] This PR has been included in build cluster-node-tuning-operator-container-v4.16.0-202402011041.p0.ge225a86.assembly.stream for distgit cluster-node-tuning-operator. |
…penshift#932) * Enhance render sync to include bootstrap rendering tests * make render-sync (cherry picked from commit e225a86)
…penshift#932) * Enhance render sync to include bootstrap rendering tests * make render-sync (cherry picked from commit e225a86)
…ner reference ehnancements (#989) * NO-JIRA: perfprof: render: make ownerReference optional (#907) * perfprof: render: make ownerReference optional When we set OwnerReference for dependant object, we must use the object UID the apiserver set. Apiserver owns this field (obviously) so setting it when missing, like the code did in the past, is wrong and will cause conflicts on updates. I the pre-render flow we can't wait for the apiserver to set a UID, so the only possible option is to not set any OwnerReference at all. Signed-off-by: Francesco Romani <fromani@redhat.com> * perfprof: render: add weak owner reference support There are cases on which we can't use the real owner reference in the render flow, because: - we don't own the performanceprofile uid - we can't wait to learn back the apiserver-decided uid because we need to render all the manifest at once. so, to group objects together and enable to safely and easily rebuild the ownerReference graph later, we add an annotation which serves as weaker owner reference, using the profile name and adding it to all the related objects. Note this form of weak reference is meant to serve to a placeholder for the real ownerReference which should be added by the relevant controller later on, when the cluster is started and is running. Signed-off-by: Francesco Romani <fromani@redhat.com> --------- Signed-off-by: Francesco Romani <fromani@redhat.com> (cherry picked from commit 0e31943) (cherry picked from commit 5308f47) * release-4.14: render: align expected machineconfig the backported expected machineconfig output was including the changes in 4.15, we need to update with the 4.14-specific content, excluding unrelated changes. Signed-off-by: Francesco Romani <fromani@redhat.com> * render: perfprofile: don't annotate perfprof (#951) don't add weak ref to the perfprof and don't write back an updated version in output. Signed-off-by: Francesco Romani <fromani@redhat.com> (cherry picked from commit fd8ea4e) (cherry picked from commit 34926bd) * e2e: testdata: remove the annotated profile after #951 merged, we don't need these anymore Signed-off-by: Francesco Romani <fromani@redhat.com> (cherry picked from commit 0d157b5) * CNF-11145: Enhance render sync to include bootstrap rendering tests (#932) * Enhance render sync to include bootstrap rendering tests * make render-sync (cherry picked from commit e225a86) * Add support to inject owner-ref argument to render command (#960) We need this change because we want to use also non default values for the owner-ref like the value none. (cherry picked from commit 631e03c) * release-4.14: hack: branch-specific updates remove lines not relevant for this branch Signed-off-by: Francesco Romani <fromani@redhat.com> --------- Signed-off-by: Francesco Romani <fromani@redhat.com> Co-authored-by: Martin Sivák <msivak@redhat.com> Co-authored-by: Ronny Baturov <rbaturov@redhat.com>
No description provided.