Skip to content
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

Add "AwaitingVDDK" back to condition reason. #1816

Merged
merged 4 commits into from Jun 22, 2021
Merged

Add "AwaitingVDDK" back to condition reason. #1816

merged 4 commits into from Jun 22, 2021

Conversation

mrnold
Copy link
Contributor

@mrnold mrnold commented Jun 2, 2021

What this PR does / why we need it:
When waiting for the PVC to be bound, copy the PVC's condition reason to the DataVolume instead of just "Pending".

Which issue(s) this PR fixes:
Fixes BZ#1965181

Release note:

Copy AwaitingVDDK condition reason to DV when PVC is waiting for v2v-vmware ConfigMap to be created.

@kubevirt-bot kubevirt-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Jun 2, 2021
@kubevirt-bot kubevirt-bot requested review from awels and maya-r June 2, 2021 21:47
@mrnold
Copy link
Contributor Author

mrnold commented Jun 4, 2021

/test pull-cdi-linter

@awels
Copy link
Member

awels commented Jun 4, 2021

Seems like this test is failing in all the lanes: Preallocation All import paths should contain Preallocation step VddkImport

@mrnold
Copy link
Contributor Author

mrnold commented Jun 4, 2021

Seems like this test is failing in all the lanes: Preallocation All import paths should contain Preallocation step VddkImport

Yes, I think it's because I removed the v2v-vmware ConfigMap for the test and forgot to put it back. I'm trying out a fix now.

awels
awels approved these changes Jun 8, 2021
View reviewed changes
Copy link
Member

@awels awels left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/approve

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Jun 8, 2021
@kubevirt-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: awels

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubevirt-bot kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 8, 2021
@awels
Copy link
Member

awels commented Jun 8, 2021

/test pull-containerized-data-importer-e2e-k8s-1.19-ceph

@mrnold
Copy link
Contributor Author

mrnold commented Jun 8, 2021

/retest

1 similar comment
@mrnold
Copy link
Contributor Author

mrnold commented Jun 11, 2021

/retest

@mrnold
Move AwaitingVDDK constant to common.
b836b1b 
Signed-off-by: Matthew Arnold <marnold@redhat.com>
@mrnold
Copy pending PVC bound condition reason to DV.
c8083d2 
Replace the fixed "Pending" string and tweak the unit test that checked
this.

Signed-off-by: Matthew Arnold <marnold@redhat.com>
@mrnold
Add a functional test for AwaitingVDDK.
cf5f0b1 
Delete v2v-vmware ConfigMap and create a DataVolume, and the bound
condition should have a reason of "AwaitingVDDK".

Signed-off-by: Matthew Arnold <marnold@redhat.com>
@mrnold
Move AwaitingVDDK to its own functional test.
d1e414a 
Retain common test code with existing table, but tack on an extra
cleanup step so v2v-vmware ConfigMap can be restored afterward.

Signed-off-by: Matthew Arnold <marnold@redhat.com>
@kubevirt-bot kubevirt-bot removed the lgtm Indicates that a PR is ready to be merged. label Jun 12, 2021
@brybacki
Copy link
Contributor

/retest

@awels
Copy link
Member

awels commented Jun 21, 2021

/lgtm

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Jun 21, 2021
@mrnold
Copy link
Contributor Author

mrnold commented Jun 21, 2021

/test pull-containerized-data-importer-e2e-k8s-1.20-upg

@mrnold
Copy link
Contributor Author

mrnold commented Jun 22, 2021

/retest

@mrnold
Copy link
Contributor Author

mrnold commented Jun 22, 2021

/test pull-containerized-data-importer-e2e-k8s-1.20-upg

@mrnold
Copy link
Contributor Author

mrnold commented Jun 22, 2021

/test pull-containerized-data-importer-e2e-k8s-1.19-ceph

@mrnold
Copy link
Contributor Author

mrnold commented Jun 22, 2021

/test pull-containerized-data-importer-e2e-k8s-1.20-hpp

@kubevirt-bot kubevirt-bot merged commit 56cfd15 into kubevirt:main Jun 22, 2021
@ghost
Copy link

ghost commented Jul 5, 2021

/cherry-pick release-v1.28
/cherry-pick release-v1.34

@kubevirt-bot
Copy link
Contributor

@fdupont-redhat: #1816 failed to apply on top of branch "release-v1.28":

error: Failed to merge in the changes.
Using index info to reconstruct a base tree...
M	pkg/common/common.go
M	pkg/controller/import-controller.go
M	pkg/controller/import-controller_test.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/controller/import-controller_test.go
CONFLICT (content): Merge conflict in pkg/controller/import-controller_test.go
Auto-merging pkg/controller/import-controller.go
CONFLICT (content): Merge conflict in pkg/controller/import-controller.go
Auto-merging pkg/common/common.go
Patch failed at 0001 Move AwaitingVDDK constant to common.

In response to this:

/cherry-pick release-v1.28
/cherry-pick release-v1.34

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.

@mrnold
Copy link
Contributor Author

mrnold commented Jul 6, 2021

/cherry-pick release-v1.34

@kubevirt-bot
Copy link
Contributor

@mrnold: only kubevirt org members may request cherry picks. You can still do the cherry-pick manually.

In response to this:

/cherry-pick release-v1.34

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.

@maya-r
Copy link
Contributor

maya-r commented Jul 6, 2021

/cherry-pick release-v1.34

@kubevirt-bot kubevirt-bot mentioned this pull request Jul 6, 2021
Merged
@kubevirt-bot
Copy link
Contributor

@maya-r: new pull request created: #1852

In response to this:

/cherry-pick release-v1.34

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.

mrnold added a commit to mrnold/containerized-data-importer that referenced this pull request Jul 6, 2021
@mrnold
Add "AwaitingVDDK" back to condition reason. (kubevirt#1816)
71bb099 
* Move AwaitingVDDK constant to common.

Signed-off-by: Matthew Arnold <marnold@redhat.com>

* Copy pending PVC bound condition reason to DV.

Replace the fixed "Pending" string and tweak the unit test that checked
this.

Signed-off-by: Matthew Arnold <marnold@redhat.com>

* Add a functional test for AwaitingVDDK.

Delete v2v-vmware ConfigMap and create a DataVolume, and the bound
condition should have a reason of "AwaitingVDDK".

Signed-off-by: Matthew Arnold <marnold@redhat.com>

* Move AwaitingVDDK to its own functional test.

Retain common test code with existing table, but tack on an extra
cleanup step so v2v-vmware ConfigMap can be restored afterward.

Signed-off-by: Matthew Arnold <marnold@redhat.com>
@mrnold mrnold mentioned this pull request Jul 6, 2021
Merged
kubevirt-bot pushed a commit that referenced this pull request Jul 7, 2021
@mrnold
[release-v1.28] Add "AwaitingVDDK" back to condition reason. (#1853)
3f2b8f8 
* Add error to DV when VDDK configmap is missing. (#1627)

Assists resolution of BZ#1886566. Use existing mechanism to copy certain
PVC annotations into DV status conditions.

Signed-off-by: Matthew Arnold <marnold@redhat.com>

* Add "AwaitingVDDK" back to condition reason. (#1816)

* Move AwaitingVDDK constant to common.

Signed-off-by: Matthew Arnold <marnold@redhat.com>

* Copy pending PVC bound condition reason to DV.

Replace the fixed "Pending" string and tweak the unit test that checked
this.

Signed-off-by: Matthew Arnold <marnold@redhat.com>

* Add a functional test for AwaitingVDDK.

Delete v2v-vmware ConfigMap and create a DataVolume, and the bound
condition should have a reason of "AwaitingVDDK".

Signed-off-by: Matthew Arnold <marnold@redhat.com>

* Move AwaitingVDDK to its own functional test.

Retain common test code with existing table, but tack on an extra
cleanup step so v2v-vmware ConfigMap can be restored afterward.

Signed-off-by: Matthew Arnold <marnold@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@awels awels awels approved these changes

@maya-r maya-r Awaiting requested review from maya-r

Assignees

@awels awels

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@mrnold @awels @kubevirt-bot @brybacki @maya-r