Ensure priority class is assigned to pod populating pvc prime #2864
Conversation
Priority class was not being passed to the pvc prime from the PVC and thus was not being added to the importer pod populating the pvc prime. There is a list of allowed annotations that can be passed and the priority class annotation was not in it. This commit adds the annotation to the allowed list. Cleaned up unneed log argument to a function related to passing the annotations. Signed-off-by: Alexander Wels <awels@redhat.com>
kubevirt-bot
added
release-note
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: akalenyu 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 |
kubevirt-bot
added
the
approved
| @@ -259,7 +259,7 @@ func (r *ReconcilerBase) reconcile(req reconcile.Request, populator populatorCon | |||
|
|
|||
| // We first perform the common reconcile steps. | |||
| // We should only continue if we get a valid PVC' | |||
| pvcPrime, err := r.reconcileCommon(pvc, populator, log) | |||
There was a problem hiding this comment.
Should we remove the log passed to reconcile too? I don't remember if I added this for some reason or just by mistake.
There was a problem hiding this comment.
So I just checked, and the log passed in has a WithValues so it might actually make sense to keep everything due to that. The log will contain the PVC name and namespace in the log line.
There was a problem hiding this comment.
Yeah that's it, I think we should keep it.
There was a problem hiding this comment.
Lets see if we can make it clearer this is the case otherwise I will do this again. I was like why are we passing this log, the struct has a logger.
as the log message. Modified the name to make it clearer this is the case. Signed-off-by: Alexander Wels <awels@redhat.com>
|
/test pull-containerized-data-importer-e2e-destructive |
| @@ -3006,7 +3006,6 @@ var _ = Describe("[vendor:cnv-qe@redhat.com][level:component]DataVolume tests", | |||
| dataVolume := utils.NewDataVolumeWithHTTPImport(dataVolumeName, "1Gi", fmt.Sprintf(utils.TinyCoreQcow2URLRateLimit, f.CdiInstallNs)) | |||
| By(fmt.Sprintf("creating new datavolume %s with priority class", dataVolume.Name)) | |||
| dataVolume.Spec.PriorityClassName = "system-cluster-critical" | |||
| dataVolume.Annotations[controller.AnnPodRetainAfterCompletion] = "true" | |||
There was a problem hiding this comment.
Maybe remove the annotation from the next two (upload and clone) tests? Other than that looks good to me.
There was a problem hiding this comment.
So the cloner is problematic, as that pod disappears too quickly and the test fails..
because the pod disappears too quickly without retain Signed-off-by: Alexander Wels <awels@redhat.com>
|
/lgtm |
|
/hold |
kubevirt-bot
added
the
do-not-merge/hold
Signed-off-by: Alexander Wels <awels@redhat.com>
|
/test pull-containerized-data-importer-e2e-nfs |
|
/lgtm |
|
/hold cancel |
kubevirt-bot
removed
the
do-not-merge/hold
|
/cherrypick release-v1.57 |
|
@awels: once the present PR merges, I will cherry-pick it on top of release-v1.57 in a new PR and assign it to you. 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 kubernetes/test-infra repository. |
|
/test pull-containerized-data-importer-e2e-hpp-previous |
|
@awels: new pull request created: #2870 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 kubernetes/test-infra repository. |
…rt#2864) * Ensure priority class is assigned to pod populating pvc prime Priority class was not being passed to the pvc prime from the PVC and thus was not being added to the importer pod populating the pvc prime. There is a list of allowed annotations that can be passed and the priority class annotation was not in it. This commit adds the annotation to the allowed list. Cleaned up unneed log argument to a function related to passing the annotations. Signed-off-by: Alexander Wels <awels@redhat.com> * Restore the logger as it was logging the pvc name as well as the log message. Modified the name to make it clearer this is the case. Signed-off-by: Alexander Wels <awels@redhat.com> * Have upload also use the populator, can't do clone because the pod disappears too quickly without retain Signed-off-by: Alexander Wels <awels@redhat.com> * Test for priority class in upload as well as fix typo in cloning test. Signed-off-by: Alexander Wels <awels@redhat.com> --------- Signed-off-by: Alexander Wels <awels@redhat.com>
…vc prime (#2872) * Ensure priority class is assigned to pod populating pvc prime (#2864) * Ensure priority class is assigned to pod populating pvc prime Priority class was not being passed to the pvc prime from the PVC and thus was not being added to the importer pod populating the pvc prime. There is a list of allowed annotations that can be passed and the priority class annotation was not in it. This commit adds the annotation to the allowed list. Cleaned up unneed log argument to a function related to passing the annotations. Signed-off-by: Alexander Wels <awels@redhat.com> * Restore the logger as it was logging the pvc name as well as the log message. Modified the name to make it clearer this is the case. Signed-off-by: Alexander Wels <awels@redhat.com> * Have upload also use the populator, can't do clone because the pod disappears too quickly without retain Signed-off-by: Alexander Wels <awels@redhat.com> * Test for priority class in upload as well as fix typo in cloning test. Signed-off-by: Alexander Wels <awels@redhat.com> --------- Signed-off-by: Alexander Wels <awels@redhat.com> * Use table import due to 1.57 using ginkgo v1 Signed-off-by: Alexander Wels <awels@redhat.com> --------- Signed-off-by: Alexander Wels <awels@redhat.com>
What this PR does / why we need it:
Priority class was not being passed to the pvc prime from the PVC and thus was not being added to the importer pod populating the pvc prime. There is a list of allowed annotations that can be passed and the priority class annotation was not in it. This commit adds the annotation to the allowed list.
Cleaned up unneeded log argument to a function related to passing the annotations.
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes RHBZ: 2231839
Special notes for your reviewer:
Release note: