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.

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

Let the operator remove virt-controller generated traces #3006

Merged
merged 1 commit into from Feb 6, 2020

Conversation

rmohr
Copy link
Member

@rmohr rmohr commented Jan 20, 2020

What this PR does / why we need it:

virt-api registers apiservices and webhooks which it does not delete. On
uninstall during the operator this lead to incomplete uninstalls which
could block future namespace deletions.

While letting still virt-api creating all these objects, let
virt-operator now manage their removal. The control is still not fully
handed over to virt-operator since there are still people using the
manifest-only installation path without the operator.

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 #1491

Special notes for your reviewer:

Release note:

Uninstall via the operator catches now all artifacts. Deleted namespaces should no longer be stuck forever after uninstalling kubevirt correctly.

@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. size/L labels Jan 20, 2020
@rmohr
Copy link
Member Author

rmohr commented Jan 20, 2020

/hold

Functional test is there but some unit tests are missing.

@kubevirt-bot kubevirt-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 20, 2020
Copy link
Member

@danielBelenky danielBelenky left a comment

Choose a reason for hiding this comment

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

Generally lgtm, just minor questions

pkg/virt-api/api.go Show resolved Hide resolved
@danielBelenky danielBelenky self-assigned this Jan 20, 2020
@kubevirt-bot kubevirt-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 21, 2020
virt-api registers apiservices and webhooks which it does not delete. On
uninstall during the operator this lead to incomplete uninstalls which
could block future namespace deletions.

While letting still virt-api creating all these objects, let
virt-operator now manage their removal. The control is still not fully
handed over to virt-operator since there are still people using the
manifest-only installation path without the operator.

Signed-off-by: Roman Mohr <rmohr@redhat.com>
@rmohr rmohr force-pushed the fix-uninstall-with-operator branch from e7666a8 to e61863c Compare January 22, 2020 11:15
@kubevirt-bot kubevirt-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 22, 2020
@rmohr
Copy link
Member Author

rmohr commented Jan 22, 2020

/hold cancel

@kubevirt-bot kubevirt-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 22, 2020
@@ -873,6 +873,18 @@ spec:
By("Sanity Checking Deployments infrastructure is deleted")
sanityCheckDeploymentsDeleted()

By("ensuring that namespaces can be successfully created and deleted")
Copy link
Member Author

Choose a reason for hiding this comment

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

This should in future prevent that we leave anything behind which can block namespace management.

@rmohr
Copy link
Member Author

rmohr commented Feb 3, 2020

/test pull-kubevirt-e2e-kind-k8s-sriov-1.14.2

@danielBelenky
Copy link
Member

/lgtm

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Feb 3, 2020
@rmohr
Copy link
Member Author

rmohr commented Feb 3, 2020

/approve

@danielBelenky thx for the review.

@kubevirt-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rmohr

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 Feb 3, 2020
@kubevirt-commenter-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

5 similar comments
@kubevirt-commenter-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@kubevirt-commenter-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@kubevirt-commenter-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@kubevirt-commenter-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@kubevirt-commenter-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@kubevirt-commenter-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@kubevirt-bot kubevirt-bot merged commit 962ac47 into kubevirt:master Feb 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Uninstall/cleanup procedures
4 participants