Remove configmap kubevirt-storage-class-defaults #1829

brybacki · 2022-03-23T13:18:14Z

Reviewer Checklist

Reviewers are supposed to review the PR for every aspect below one by one. To check an item means the PR is either "OK" or "Not Applicable" in terms of that item. All items are supposed to be checked before merging a PR.

ConfigMap kubevirt-storage-class-defaults is no longer needed for kubevirt and cdi. It is now replaced by the CDI StorageProfile resources.

This PR removes the code that creates and reconciles kubevirt-storage-class-defaults. Special cluster roles for accessing this configmap are also removed.

Release note:

Remove config map kubevirt-storage-class-defaults

hco-bot · 2022-03-23T14:46:47Z

hco-e2e-upgrade-prev-index-sno-azure lane succeeded.
/override ci/prow/hco-e2e-upgrade-prev-index-sno-aws

kubevirt-bot · 2022-03-23T14:47:00Z

@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-upgrade-prev-index-sno-aws

In response to this:

hco-e2e-upgrade-prev-index-sno-azure lane succeeded.
/override ci/prow/hco-e2e-upgrade-prev-index-sno-aws

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.

hco-bot · 2022-03-23T15:34:34Z

hco-e2e-image-index-azure, hco-e2e-image-index-gcp lanes succeeded.
/override ci/prow/hco-e2e-image-index-aws
hco-e2e-image-index-sno-azure lane succeeded.
/override ci/prow/hco-e2e-image-index-sno-aws

hco-bot · 2022-03-23T15:47:19Z

okd-hco-e2e-image-index-gcp lane succeeded.
/override ci/prow/okd-hco-e2e-image-index-aws
hco-e2e-image-index-azure, hco-e2e-image-index-gcp lanes succeeded.
/override ci/prow/hco-e2e-image-index-aws
hco-e2e-image-index-sno-azure lane succeeded.
/override ci/prow/hco-e2e-image-index-sno-aws

hco-bot · 2022-03-23T15:55:50Z

okd-hco-e2e-image-index-gcp lane succeeded.
/override ci/prow/okd-hco-e2e-image-index-aws
hco-e2e-image-index-azure, hco-e2e-image-index-gcp lanes succeeded.
/override ci/prow/hco-e2e-image-index-aws
hco-e2e-image-index-sno-azure lane succeeded.
/override ci/prow/hco-e2e-image-index-sno-aws

hco-bot · 2022-03-23T16:17:46Z

okd-hco-e2e-image-index-gcp lane succeeded.
/override ci/prow/okd-hco-e2e-image-index-aws
hco-e2e-image-index-azure, hco-e2e-image-index-gcp lanes succeeded.
/override ci/prow/hco-e2e-image-index-aws
hco-e2e-image-index-sno-azure lane succeeded.
/override ci/prow/hco-e2e-image-index-sno-aws

kubevirt-bot · 2022-03-23T16:26:25Z

@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-image-index-aws, ci/prow/hco-e2e-image-index-sno-aws

In response to this:

hco-e2e-image-index-azure, hco-e2e-image-index-gcp lanes succeeded.
/override ci/prow/hco-e2e-image-index-aws
hco-e2e-image-index-sno-azure lane succeeded.
/override ci/prow/hco-e2e-image-index-sno-aws

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.

kubevirt-bot · 2022-03-23T16:31:36Z

@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-image-index-aws, ci/prow/hco-e2e-image-index-sno-aws, ci/prow/okd-hco-e2e-image-index-aws

In response to this:

okd-hco-e2e-image-index-gcp lane succeeded.
/override ci/prow/okd-hco-e2e-image-index-aws
hco-e2e-image-index-azure, hco-e2e-image-index-gcp lanes succeeded.
/override ci/prow/hco-e2e-image-index-aws
hco-e2e-image-index-sno-azure lane succeeded.
/override ci/prow/hco-e2e-image-index-sno-aws

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.

kubevirt-bot · 2022-03-23T16:35:33Z

@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-image-index-aws, ci/prow/hco-e2e-image-index-sno-aws, ci/prow/okd-hco-e2e-image-index-aws

In response to this:

okd-hco-e2e-image-index-gcp lane succeeded.
/override ci/prow/okd-hco-e2e-image-index-aws
hco-e2e-image-index-azure, hco-e2e-image-index-gcp lanes succeeded.
/override ci/prow/hco-e2e-image-index-aws
hco-e2e-image-index-sno-azure lane succeeded.
/override ci/prow/hco-e2e-image-index-sno-aws

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.

kubevirt-bot · 2022-03-23T16:46:06Z

@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-image-index-aws, ci/prow/hco-e2e-image-index-sno-aws, ci/prow/okd-hco-e2e-image-index-aws

In response to this:

okd-hco-e2e-image-index-gcp lane succeeded.
/override ci/prow/okd-hco-e2e-image-index-aws
hco-e2e-image-index-azure, hco-e2e-image-index-gcp lanes succeeded.
/override ci/prow/hco-e2e-image-index-aws
hco-e2e-image-index-sno-azure lane succeeded.
/override ci/prow/hco-e2e-image-index-sno-aws

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.

coveralls · 2022-03-24T14:35:16Z

Pull Request Test Coverage Report for Build 2147463562

4 of 4 (100.0%) changed or added relevant lines in 1 file are covered.
1 unchanged line in 1 file lost coverage.
Overall coverage decreased (-0.2%) to 84.233%

Files with Coverage Reduction	New Missed Lines	%
controllers/operands/operandHandler.go	1	88.02%

Totals
Change from base Build 2147212234:	-0.2%
Covered Lines:	4071
Relevant Lines:	4833

💛 - Coveralls

hco-bot · 2022-03-24T15:36:25Z

hco-e2e-upgrade-index-azure lane succeeded.
/override ci/prow/hco-e2e-upgrade-index-aws
hco-e2e-upgrade-prev-index-sno-azure lane succeeded.
/override ci/prow/hco-e2e-upgrade-prev-index-sno-aws
hco-e2e-upgrade-index-sno-azure lane succeeded.
/override ci/prow/hco-e2e-upgrade-index-sno-aws
hco-e2e-image-index-sno-azure lane succeeded.
/override ci/prow/hco-e2e-image-index-sno-aws

kubevirt-bot · 2022-03-24T15:36:30Z

@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-image-index-sno-aws, ci/prow/hco-e2e-upgrade-index-aws, ci/prow/hco-e2e-upgrade-index-sno-aws, ci/prow/hco-e2e-upgrade-prev-index-sno-aws

In response to this:

hco-e2e-upgrade-index-azure lane succeeded.
/override ci/prow/hco-e2e-upgrade-index-aws
hco-e2e-upgrade-prev-index-sno-azure lane succeeded.
/override ci/prow/hco-e2e-upgrade-prev-index-sno-aws
hco-e2e-upgrade-index-sno-azure lane succeeded.
/override ci/prow/hco-e2e-upgrade-index-sno-aws
hco-e2e-image-index-sno-azure lane succeeded.
/override ci/prow/hco-e2e-image-index-sno-aws

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.

hco-bot · 2022-03-24T15:47:52Z

hco-e2e-image-index-azure lane succeeded.
/override ci/prow/hco-e2e-image-index-gcp
hco-e2e-image-index-azure lane succeeded.
/override ci/prow/hco-e2e-image-index-aws
okd-hco-e2e-image-index-gcp lane succeeded.
/override ci/prow/okd-hco-e2e-image-index-aws
hco-e2e-upgrade-prev-index-azure lane succeeded.
/override ci/prow/hco-e2e-upgrade-prev-index-aws

hco-bot · 2022-03-24T15:56:43Z

hco-e2e-image-index-azure lane succeeded.
/override ci/prow/hco-e2e-image-index-gcp
hco-e2e-image-index-azure lane succeeded.
/override ci/prow/hco-e2e-image-index-aws
okd-hco-e2e-image-index-gcp lane succeeded.
/override ci/prow/okd-hco-e2e-image-index-aws
hco-e2e-upgrade-prev-index-azure lane succeeded.
/override ci/prow/hco-e2e-upgrade-prev-index-aws

hco-bot · 2022-03-24T16:20:07Z

hco-e2e-image-index-azure lane succeeded.
/override ci/prow/hco-e2e-image-index-gcp
hco-e2e-image-index-azure lane succeeded.
/override ci/prow/hco-e2e-image-index-aws
okd-hco-e2e-image-index-gcp lane succeeded.
/override ci/prow/okd-hco-e2e-image-index-aws
hco-e2e-upgrade-prev-index-azure lane succeeded.
/override ci/prow/hco-e2e-upgrade-prev-index-aws

kubevirt-bot · 2022-03-24T16:20:13Z

@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-image-index-aws, ci/prow/hco-e2e-image-index-gcp, ci/prow/hco-e2e-upgrade-prev-index-aws, ci/prow/okd-hco-e2e-image-index-aws

In response to this:

hco-e2e-image-index-azure lane succeeded.
/override ci/prow/hco-e2e-image-index-gcp
hco-e2e-image-index-azure lane succeeded.
/override ci/prow/hco-e2e-image-index-aws
okd-hco-e2e-image-index-gcp lane succeeded.
/override ci/prow/okd-hco-e2e-image-index-aws
hco-e2e-upgrade-prev-index-azure lane succeeded.
/override ci/prow/hco-e2e-upgrade-prev-index-aws

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.

tiraboschi · 2022-03-25T08:06:59Z

in https://github.com/kubevirt/hyperconverged-cluster-operator/blob/main/assets/upgradePatches.json#L42 we have a mechanism to remove leftovers on upgrades according to a json configuration file.
Can you please add a few entries for those resources that should be deleted during upgrades?

tiraboschi

in https://github.com/kubevirt/hyperconverged-cluster-operator/blob/main/assets/upgradePatches.json#L42 we have a mechanism to remove leftovers on upgrades according to a json configuration file.
Can you please add a few entries for those resources that should be deleted during upgrades?

brybacki · 2022-03-25T08:14:44Z

in https://github.com/kubevirt/hyperconverged-cluster-operator/blob/main/assets/upgradePatches.json#L42 we have a mechanism to remove leftovers on upgrades according to a json configuration file. Can you please add a few entries for those resources that should be deleted during upgrades?

Looking into this now.

kubevirt-bot · 2022-04-08T20:32:42Z

@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-kv-smoke-azure

In response to this:

hco-e2e-kv-smoke-gcp lane succeeded.
/override ci/prow/hco-e2e-kv-smoke-azure

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.

tiraboschi · 2022-04-09T08:04:51Z

/retest

tiraboschi · 2022-04-09T18:28:34Z

/retest

tiraboschi · 2022-04-10T11:41:36Z

/retest

tiraboschi · 2022-04-10T15:27:08Z

/retest

tiraboschi · 2022-04-11T07:18:53Z

this is still failing due to leftovers on relatedObjects:

  relatedObjects:
...
  - apiVersion: rbac.authorization.k8s.io/v1
    kind: Role
    name: hco.kubevirt.io:config-reader
    namespace: kubevirt-hyperconverged
    resourceVersion: "35479"
    uid: 4e281b39-5ba9-4f26-bf17-20e160ed599f
  - apiVersion: rbac.authorization.k8s.io/v1
    kind: RoleBinding
    name: hco.kubevirt.io:config-reader
    namespace: kubevirt-hyperconverged
    resourceVersion: "35612"
    uid: ea5ec7d6-6e59-4990-830c-6166ac1d83e1

ConfigMap kubevirt-storage-class-defaults is no longer needed for kubevirt and cdi. It is now replaced by the CDI StorageProfile resources. Signed-off-by: Bartosz Rybacki <brybacki@redhat.com>

Signed-off-by: Bartosz Rybacki <brybacki@redhat.com>

Also remove related resources that are no longer needed Signed-off-by: Bartosz Rybacki <brybacki@redhat.com>

Signed-off-by: Bartosz Rybacki <brybacki@redhat.com>

sonarcloud · 2022-04-11T09:08:39Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

tiraboschi · 2022-04-11T11:08:36Z

/override coverage/coveralls
tolerating slightly decreased coverage

kubevirt-bot · 2022-04-11T11:08:38Z

@tiraboschi: Overrode contexts on behalf of tiraboschi: coverage/coveralls

In response to this:

/override coverage/coveralls
tolerating slightly decreased coverage

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 · 2022-04-11T11:38:59Z

@brybacki: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/hco-e2e-kv-smoke-gcp	`adc324a`	link	true	`/test hco-e2e-kv-smoke-gcp`

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.

hco-bot · 2022-04-11T11:56:02Z

hco-e2e-kv-smoke-azure lane succeeded.
/override ci/prow/hco-e2e-kv-smoke-gcp

kubevirt-bot · 2022-04-11T11:56:05Z

@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-kv-smoke-gcp

In response to this:

hco-e2e-kv-smoke-azure lane succeeded.
/override ci/prow/hco-e2e-kv-smoke-gcp

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.

nunnatsa · 2022-04-11T13:18:38Z

/lgtm
/approve

kubevirt-bot · 2022-04-11T13:22:10Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nunnatsa

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:

~~OWNERS~~ [nunnatsa]

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

The previous code was trying to remove a reference (relatedObject) to an object from the status only directly after its removal. If any error happens in the middle, and the object was really destroyed but releatedObject reference not correctly removed, the operator wasn't going to retry on the next loop. Let's make it more robust always retrying to clean releatedObjects if needed. This is a partial cherry pick of a change implemented in kubevirt#1829 Signed-off-by: Simone Tiraboschi <stirabos@redhat.com>

The previous code was trying to remove a reference (relatedObject) to an object from the status only directly after its removal. If any error happens in the middle, and the object was really destroyed but releatedObject reference not correctly removed, the operator wasn't going to retry on the next loop. Let's make it more robust always retrying to clean releatedObjects if needed. This is a partial cherry pick of a change implemented in #1829 Signed-off-by: Simone Tiraboschi <stirabos@redhat.com>

kubevirt-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. dco-signoff: no Indicates the PR's author has not DCO signed all their commits. labels Mar 23, 2022

kubevirt-bot requested review from machadovilaca and orenc1 March 23, 2022 13:18

kubevirt-bot added the size/L label Mar 23, 2022

brybacki force-pushed the remove-configmap-storageclass branch from 3e1eced to 2f99023 Compare March 23, 2022 13:20

kubevirt-bot added dco-signoff: yes Indicates the PR's author has DCO signed all their commits. and removed dco-signoff: no Indicates the PR's author has not DCO signed all their commits. labels Mar 23, 2022

brybacki force-pushed the remove-configmap-storageclass branch from f58efd6 to 2dc7b69 Compare March 24, 2022 14:14

tiraboschi requested changes Mar 25, 2022

View reviewed changes

kubevirt-bot assigned tiraboschi Mar 25, 2022

kubevirt-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 10, 2022

tiraboschi force-pushed the remove-configmap-storageclass branch from eae2b5c to efefa32 Compare April 10, 2022 09:48

kubevirt-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 10, 2022

brybacki added 5 commits April 11, 2022 09:52

Do not create StorageConfig

a561819

ConfigMap kubevirt-storage-class-defaults is no longer needed for kubevirt and cdi. It is now replaced by the CDI StorageProfile resources. Signed-off-by: Bartosz Rybacki <brybacki@redhat.com>

Remove unneeded test and description

78b27b8

Signed-off-by: Bartosz Rybacki <brybacki@redhat.com>

Remove storage class configmap during update

09d6cab

Also remove related resources that are no longer needed Signed-off-by: Bartosz Rybacki <brybacki@redhat.com>

Correct version in upgrade cleanup

119d0b1

Signed-off-by: Bartosz Rybacki <brybacki@redhat.com>

Test object removal on upgrade

adc324a

Signed-off-by: Bartosz Rybacki <brybacki@redhat.com>

tiraboschi force-pushed the remove-configmap-storageclass branch from efefa32 to adc324a Compare April 11, 2022 09:06

kubevirt-bot assigned nunnatsa Apr 11, 2022

kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Apr 11, 2022

kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 11, 2022

kubevirt-bot merged commit f6664b6 into kubevirt:main Apr 11, 2022

tiraboschi mentioned this pull request Apr 12, 2022

[release-1.6] Make the removal of relatedObjects more robust #1896

Merged

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove configmap kubevirt-storage-class-defaults #1829

Remove configmap kubevirt-storage-class-defaults #1829

brybacki commented Mar 23, 2022 •

edited

Loading

hco-bot commented Mar 23, 2022

kubevirt-bot commented Mar 23, 2022

hco-bot commented Mar 23, 2022

hco-bot commented Mar 23, 2022

hco-bot commented Mar 23, 2022

hco-bot commented Mar 23, 2022

kubevirt-bot commented Mar 23, 2022

kubevirt-bot commented Mar 23, 2022

kubevirt-bot commented Mar 23, 2022

kubevirt-bot commented Mar 23, 2022

coveralls commented Mar 24, 2022 •

edited

Loading

hco-bot commented Mar 24, 2022

kubevirt-bot commented Mar 24, 2022

hco-bot commented Mar 24, 2022

hco-bot commented Mar 24, 2022

hco-bot commented Mar 24, 2022

kubevirt-bot commented Mar 24, 2022

tiraboschi commented Mar 25, 2022

tiraboschi left a comment

brybacki commented Mar 25, 2022

kubevirt-bot commented Apr 8, 2022

tiraboschi commented Apr 9, 2022

tiraboschi commented Apr 9, 2022

tiraboschi commented Apr 10, 2022

tiraboschi commented Apr 10, 2022

tiraboschi commented Apr 11, 2022

sonarcloud bot commented Apr 11, 2022

tiraboschi commented Apr 11, 2022

kubevirt-bot commented Apr 11, 2022

openshift-ci bot commented Apr 11, 2022

hco-bot commented Apr 11, 2022

kubevirt-bot commented Apr 11, 2022

nunnatsa commented Apr 11, 2022

kubevirt-bot commented Apr 11, 2022

Remove configmap kubevirt-storage-class-defaults #1829

Remove configmap kubevirt-storage-class-defaults #1829

Conversation

brybacki commented Mar 23, 2022 • edited Loading

hco-bot commented Mar 23, 2022

kubevirt-bot commented Mar 23, 2022

hco-bot commented Mar 23, 2022

hco-bot commented Mar 23, 2022

hco-bot commented Mar 23, 2022

hco-bot commented Mar 23, 2022

kubevirt-bot commented Mar 23, 2022

kubevirt-bot commented Mar 23, 2022

kubevirt-bot commented Mar 23, 2022

kubevirt-bot commented Mar 23, 2022

coveralls commented Mar 24, 2022 • edited Loading

Pull Request Test Coverage Report for Build 2147463562

💛 - Coveralls

hco-bot commented Mar 24, 2022

kubevirt-bot commented Mar 24, 2022

hco-bot commented Mar 24, 2022

hco-bot commented Mar 24, 2022

hco-bot commented Mar 24, 2022

kubevirt-bot commented Mar 24, 2022

tiraboschi commented Mar 25, 2022

tiraboschi left a comment

Choose a reason for hiding this comment

brybacki commented Mar 25, 2022

kubevirt-bot commented Apr 8, 2022

tiraboschi commented Apr 9, 2022

tiraboschi commented Apr 9, 2022

tiraboschi commented Apr 10, 2022

tiraboschi commented Apr 10, 2022

tiraboschi commented Apr 11, 2022

sonarcloud bot commented Apr 11, 2022

tiraboschi commented Apr 11, 2022

kubevirt-bot commented Apr 11, 2022

openshift-ci bot commented Apr 11, 2022

hco-bot commented Apr 11, 2022

kubevirt-bot commented Apr 11, 2022

nunnatsa commented Apr 11, 2022

kubevirt-bot commented Apr 11, 2022

brybacki commented Mar 23, 2022 •

edited

Loading

coveralls commented Mar 24, 2022 •

edited

Loading