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

OCPBUGSM-29066 Remove cluster resources after clusterdeployment deletion #1801

Merged

Conversation

masayag
Copy link
Contributor

@masayag masayag commented May 23, 2021

When cluster is deregistered, either by clusterdeployment or by the API,
the cluster record is softly deleted, meaning the attribute deleted_at
in the DB is being set.
It is the responsiblity of the garbage collector to run periodically and
remove all of the cluster's resources.
For that purpose, the garbage collection should be enabled for clearing
cluster resources in operator deployment as well.

Signed-off-by: Moti Asayag masayag@redhat.com

@openshift-ci openshift-ci bot requested review from nmagnezi and omertuc May 23, 2021 13:04
@filanov
Copy link
Contributor

filanov commented May 25, 2021

You could a subsystem test that delete a cluster and check for example that the events table in the DB is empty.
If you are doing that then you will need to change the interval of this GC to 5 sec or something like that.

db.Model(&cluster).UpdateColumn("updated_at", time.Now().AddDate(-1, 0, 0))

By("Wait 5 seconds to let garbage collector be triggered")
time.Sleep(5 * time.Second)
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of waiting you could use Eventually if safer then a static sleep.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Replaced with Consistently which is useful in this context.

db.Model(&cluster).UpdateColumn("deleted_at", time.Now().AddDate(-1, 0, 0))

By("Wait 10 seconds to let garbage collector be triggered and finalize permenant removal")
time.Sleep(10 * time.Second)
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of waiting you could use Eventually if safer then a static sleep.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

subsystem/kubeapi_test.go Outdated Show resolved Hide resolved
@masayag
Copy link
Contributor Author

masayag commented May 26, 2021

/retest

@masayag masayag requested a review from filanov May 27, 2021 08:01

By("Verify no event for deregistering the cluster was emitted")
msg := "Cluster is deregistered"
Consistently(func() []*common.Event {
Copy link
Contributor

Choose a reason for hiding this comment

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

why check consistency?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To make sure that during that period the execution of the garbageCollector.DeregisterClusterInternal did not occur and as a result the cluster was not deregistered.

Eventually(func() error {
_, err := common.GetClusterFromDBWhere(db, common.UseEagerLoading, common.IncludeDeletedRecords, "kube_key_name = ? and kube_key_namespace = ?", clusterKey.Name, clusterKey.Namespace)
return err
}, "30s", "10s").Should(HaveOccurred())
Copy link
Contributor

Choose a reason for hiding this comment

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

suggesting to increase it to a minute to be on the safe side.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do.

When cluster is deregistered, either by clusterdeployment or by the API,
the cluster record is softly deleted, meaning the  attribute 'deleted_at'
in the DB is being set.
It is the responsiblity of the garbage collector to run periodically and
remove all of the cluster's resources.
For that purpose, the garbage collection should be enabled for clearing
cluster resources in operator deployment as well.

Signed-off-by: Moti Asayag <masayag@redhat.com>
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label May 27, 2021
@openshift-ci
Copy link

openshift-ci bot commented May 27, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: filanov, masayag

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 27, 2021
@openshift-ci
Copy link

openshift-ci bot commented May 27, 2021

@masayag: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/assisted-operator-install-aws ed7afa3 link /test assisted-operator-install-aws

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.

@masayag
Copy link
Contributor Author

masayag commented May 27, 2021

/retest

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. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants