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.

Sign up for GitHub

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

Jump to bottom

Bug 1915694: Add immediate request annotaion to auploaded dvs #7822

Merged

openshift-merge-robot merged 1 commit into openshift:master from yaacov:add-immediate-requested-annotaion-to-uploaded-dv

Jan 13, 2021

Member

yaacov commented Jan 13, 2021 •

edited

Add storage.bind.immediate.requested to uploaded data volumes.

Ref:
kubevirt/containerized-data-importer#1560

Screenshoot:

openshift-ci-robot added bugzilla/severity-urgent bugzilla/valid-bug labels

Contributor

openshift-ci-robot commented Jan 13, 2021

@yaacov: This pull request references Bugzilla bug 1915694, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.7.0) matches configured target release for branch (4.7.0)
bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1915694: Add immediate request annotaion to auploaded dvs

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.

openshift-ci-robot added the component/kubevirt label

openshift-ci-robot requested review from glekner and gouyang

January 13, 2021 11:17

openshift-ci-robot added the approved label

Contributor

openshift-ci-robot commented Jan 13, 2021

@yaacov: This pull request references Bugzilla bug 1915694, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.7.0) matches configured target release for branch (4.7.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1915694: Add immediate request annotaion to auploaded dvs

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.

glekner suggested changes

View reviewed changes

frontend/packages/kubevirt-plugin/src/k8s/requests/cdi-upload/cdi-upload-requests.ts Outdated

+                // BZ-1915694: uploaded DV must have "storage.bind.immediate.requested" annotation.
+                dataVolume.metadata.annotations = {
+                  ...dataVolume.metadata.annotations,
+                  'storage.bind.immediate.requested': 'true',

Contributor

glekner Jan 13, 2021

can you create a const for it in src/components/cdi-upload-provider/consts.ts

Member Author

yaacov Jan 13, 2021

sure +1

frontend/packages/kubevirt-plugin/src/k8s/requests/cdi-upload/cdi-upload-requests.ts Outdated

               export const createUploadPVC = async (dataVolume: V1alpha1DataVolume) => {
                 const dataVolumeName = getName(dataVolume);
                 const namespace = getNamespace(dataVolume);
+                // BZ-1915694: uploaded DV must have "storage.bind.immediate.requested" annotation.

Contributor

glekner Jan 13, 2021

dont think we need a comment

Member Author

yaacov Jan 13, 2021

ok

frontend/packages/kubevirt-plugin/src/k8s/requests/cdi-upload/cdi-upload-requests.ts Outdated

               export const createUploadPVC = async (dataVolume: V1alpha1DataVolume) => {
                 const dataVolumeName = getName(dataVolume);
                 const namespace = getNamespace(dataVolume);
+                // BZ-1915694: uploaded DV must have "storage.bind.immediate.requested" annotation.
+                dataVolume.metadata.annotations = {
+                  ...dataVolume.metadata.annotations,

Contributor

glekner Jan 13, 2021

Suggested change

      
                ...dataVolume.metadata.annotations,
          
                ...dataVolume?.metadata?.annotations,

Member Author

yaacov Jan 13, 2021

+1

Contributor

rawagner Jan 13, 2021

it should be

Suggested change

      
                ...dataVolume.metadata.annotations,
          
                ...(dataVolume?.metadata?.annotations || []),

rawagner suggested changes

View reviewed changes

frontend/packages/kubevirt-plugin/src/k8s/requests/cdi-upload/cdi-upload-requests.ts

               export const createUploadPVC = async (dataVolume: V1alpha1DataVolume) => {
                 const dataVolumeName = getName(dataVolume);
                 const namespace = getNamespace(dataVolume);
+                // BZ-1915694: uploaded DV must have "storage.bind.immediate.requested" annotation.
+                dataVolume.metadata.annotations = {

Contributor

rawagner Jan 13, 2021

we will need the same for add-template-source.tsx

Member Author

yaacov Jan 13, 2021

add-template-source.tsx calls this function in case of upload, so it's safe

rawagner suggested changes

View reviewed changes

frontend/packages/kubevirt-plugin/src/k8s/requests/cdi-upload/cdi-upload-requests.ts Outdated

               export const createUploadPVC = async (dataVolume: V1alpha1DataVolume) => {
                 const dataVolumeName = getName(dataVolume);
                 const namespace = getNamespace(dataVolume);
+                // BZ-1915694: uploaded DV must have "storage.bind.immediate.requested" annotation.
+                dataVolume.metadata.annotations = {
+                  ...dataVolume.metadata.annotations,

Contributor

rawagner Jan 13, 2021

it should be

Suggested change

      
                ...dataVolume.metadata.annotations,
          
                ...(dataVolume?.metadata?.annotations || []),

yaacov force-pushed the add-immediate-requested-annotaion-to-uploaded-dv branch from e4633bb to 6c24401 Compare

January 13, 2021 11:51

Contributor

glekner commented Jan 13, 2021

/lgtm

openshift-ci-robot assigned glekner

openshift-ci-robot added the lgtm label

Contributor

glekner commented Jan 13, 2021 •

edited

/lgtm hold

openshift-ci-robot added the do-not-merge/hold label


          Add immediate request annotaion to auploaded dvs

7f20a11

yaacov force-pushed the add-immediate-requested-annotaion-to-uploaded-dv branch from 6c24401 to 7f20a11 Compare

January 13, 2021 11:56

openshift-ci-robot removed the lgtm label

Contributor

glekner commented Jan 13, 2021

/lgtm

openshift-ci-robot added the lgtm label

Contributor

openshift-ci-robot commented Jan 13, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: glekner, yaacov

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:

~~frontend/packages/kubevirt-plugin/OWNERS~~ [glekner,yaacov]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Member Author

yaacov commented Jan 13, 2021

/hold cancel

openshift-ci-robot removed the do-not-merge/hold label

openshift-merge-robot merged commit d3f5059 into openshift:master

Contributor

openshift-ci-robot commented Jan 13, 2021

@yaacov: All pull requests linked via external trackers have merged:

openshift/console#7822

Bugzilla bug 1915694 has been moved to the MODIFIED state.

In response to this:

Bug 1915694: Add immediate request annotaion to auploaded dvs

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.

spadgett added this to the v4.7 milestone

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment