[release-4.10] Bug 2099843: In tests, expect that unreferenced logical router policies will not be garbage collected #1157

kyrtapz · 2022-06-22T16:33:31Z

libovsdb server does not remove unreferenced non-root objects.
It is expected that the entries will be there in tests and this is not something that could happen in real world.

This workaround is not needed in 4.11 and newer since we are explicitly removing logical router policies without next hops:

ovn-kubernetes/go-controller/pkg/libovsdbops/router.go

Lines 352 to 359 in 27d4897

    
           // if no next-hops remain in the policy, remove it alltogether 
        
           router.Policies = append(router.Policies, lrp.UUID) 
        
           opModel := operationModel{ 
        
           	Model:       lrp, 
        
           	BulkOp:      false, 
        
           	ErrNotFound: false, 
        
           } 
        
           opModels = append(opModels, opModel)

Since it is not something that will happen in real world deployments I opted for the workaround instead of changing the removal logic at the risk of introducing a regression.

/cc @flavio-fernandes @trozet

openshift-ci · 2022-06-22T16:34:07Z

@kyrtapz: This pull request references Bugzilla bug 2099843, which is invalid:

expected the bug to target the "4.10.z" release, but it targets "---" instead
expected Bugzilla bug 2099843 to depend on a bug targeting a release in 4.11.0 and in one of the following states: VERIFIED, RELEASE_PENDING, CLOSED (ERRATA), CLOSED (CURRENTRELEASE), but no dependents were found

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

[release-4.10] Bug 2099843: In tests, expect that unreferenced LogicalRouterPolicy'ies will not be garbage collected

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.

go-controller/pkg/ovn/egressip_test.go

flavio-fernandes

/lgtm

go-controller/pkg/ovn/egressip_test.go

flavio-fernandes · 2022-06-22T21:04:07Z

libovsdb issue linked to this PR: ovn-org/libovsdb#219

openshift-bot · 2022-06-23T01:00:38Z

/bugzilla refresh

Recalculating validity in case the underlying Bugzilla bug has changed.

openshift-ci · 2022-06-23T01:00:45Z

@openshift-bot: This pull request references Bugzilla bug 2099843, which is invalid:

expected the bug to target the "4.10.z" release, but it targets "---" instead
expected Bugzilla bug 2099843 to depend on a bug targeting a release in 4.11.0 and in one of the following states: VERIFIED, RELEASE_PENDING, CLOSED (ERRATA), CLOSED (CURRENTRELEASE), but no dependents were found

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

/bugzilla refresh

Recalculating validity in case the underlying Bugzilla bug has changed.

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.

kyrtapz · 2022-06-23T14:53:33Z

/bugzilla refresh

flavio-fernandes

/lgtm

openshift-ci · 2022-06-23T14:54:35Z

@kyrtapz: This pull request references Bugzilla bug 2099843, which is valid.

6 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.10.z) matches configured target release for branch (4.10.z)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)
dependent bug Bugzilla bug 2100502 is in the state CLOSED (CURRENTRELEASE), which is one of the valid states (VERIFIED, RELEASE_PENDING, CLOSED (ERRATA), CLOSED (CURRENTRELEASE))
dependent Bugzilla bug 2100502 targets the "4.11.0" release, which is one of the valid target releases: 4.11.0
bug has dependents

Requesting review from QA contact:
/cc @anuragthehatter

In response to this:

/bugzilla refresh

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.

…renced non-root objects. Signed-off-by: Patryk Diak <pdiak@redhat.com>

openshift-ci · 2022-06-23T15:01:36Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: flavio-fernandes, kyrtapz
To complete the pull request process, please assign knobunc after the PR has been reviewed.
You can assign the PR to them by writing /assign @knobunc in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

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

openshift-ci · 2022-06-23T15:03:20Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: flavio-fernandes, kyrtapz
To complete the pull request process, please assign knobunc after the PR has been reviewed.
You can assign the PR to them by writing /assign @knobunc in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

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

go-controller/pkg/ovn/egressip_test.go

flavio-fernandes · 2022-06-23T20:10:01Z

/retest

flavio-fernandes · 2022-06-24T02:37:54Z

/retest-required

openshift-ci · 2022-06-24T04:04:33Z

@kyrtapz: The following tests 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/e2e-azure-ovn	`3bcbda9`	link	false	`/test e2e-azure-ovn`
ci/prow/e2e-vsphere-ovn	`3bcbda9`	link	false	`/test e2e-vsphere-ovn`
ci/prow/okd-e2e-gcp-ovn	`3bcbda9`	link	false	`/test okd-e2e-gcp-ovn`

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.

openshift-bot · 2022-09-22T09:01:08Z

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

openshift-bot · 2022-10-23T00:30:13Z

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale

kyrtapz · 2022-11-03T12:42:12Z

/remove-lifecycle rotten

flavio-fernandes · 2022-11-03T13:36:49Z

/assign @jcaamano

openshift-ci · 2022-11-04T10:17:29Z

@kyrtapz: This pull request references Bugzilla bug 2099843, which is valid.

6 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.10.z) matches configured target release for branch (4.10.z)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)
dependent bug Bugzilla bug 2100502 is in the state CLOSED (CURRENTRELEASE), which is one of the valid states (VERIFIED, RELEASE_PENDING, CLOSED (ERRATA), CLOSED (CURRENTRELEASE))
dependent Bugzilla bug 2100502 targets the "4.11.0" release, which is one of the valid target releases: 4.11.0, 4.11.z
bug has dependents

Requesting review from QA contact:
/cc @anuragthehatter

In response to this:

[release-4.10] Bug 2099843: In tests, expect that unreferenced logical router policies will not be garbage collected

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.

kyrtapz · 2022-11-04T15:31:57Z

/close

the workaround is no longer needed with #1228 which brought the removal of empty lrps:

159c52b#diff-5d62dde13a2856ea3d4b324968dbaeed261fcb74cb3b81909edf40a6634fa1f4R505-R512

openshift-ci · 2022-11-04T15:32:50Z

@kyrtapz: Closed this PR.

In response to this:

/close

this is not needed with #1228 which brought the removal of empty lrps:

159c52b#diff-5d62dde13a2856ea3d4b324968dbaeed261fcb74cb3b81909edf40a6634fa1f4R505-R512

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-11-04T15:32:59Z

@kyrtapz: This pull request references Bugzilla bug 2099843. The bug has been updated to no longer refer to the pull request using the external bug tracker. All external bug links have been closed. The bug has been moved to the NEW state.
Warning: Failed to comment on Bugzilla bug with reason for changed state.

In response to this:

[release-4.10] Bug 2099843: In tests, expect that unreferenced logical router policies will not be garbage collected

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 bot requested review from flavio-fernandes and trozet June 22, 2022 16:33

openshift-ci bot added bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels Jun 22, 2022

kyrtapz changed the title ~~[release-4.10] Bug 2099843: In tests, expect that unreferenced LogicalRouterPolicy'ies will not be garbage collected~~ [release-4.10] Bug 2099843: In tests, expect that unreferenced logical router policies will not be garbage collected Jun 22, 2022

flavio-fernandes reviewed Jun 22, 2022

View reviewed changes

go-controller/pkg/ovn/egressip_test.go Outdated Show resolved Hide resolved

flavio-fernandes reviewed Jun 22, 2022

View reviewed changes

go-controller/pkg/ovn/egressip_test.go Outdated Show resolved Hide resolved

openshift-ci bot assigned flavio-fernandes Jun 22, 2022

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 22, 2022

kyrtapz force-pushed the fix_eip_test branch from fe991d4 to 4cab6be Compare June 23, 2022 14:53

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jun 23, 2022

openshift-ci bot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Jun 23, 2022

flavio-fernandes reviewed Jun 23, 2022

View reviewed changes

openshift-ci bot removed the bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Jun 23, 2022

openshift-ci bot requested a review from anuragthehatter June 23, 2022 14:54

Take into account that the ovsdb used in tests does not remove unrefe…

3bcbda9

…renced non-root objects. Signed-off-by: Patryk Diak <pdiak@redhat.com>

kyrtapz force-pushed the fix_eip_test branch from 4cab6be to 3bcbda9 Compare June 23, 2022 14:55

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 23, 2022

flavio-fernandes reviewed Jun 23, 2022

View reviewed changes

go-controller/pkg/ovn/egressip_test.go Show resolved Hide resolved

openshift-ci bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 22, 2022

openshift-ci bot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Oct 23, 2022

openshift-ci bot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Nov 3, 2022

openshift-ci bot assigned jcaamano Nov 3, 2022

openshift-ci bot closed this Nov 4, 2022

kyrtapz deleted the fix_eip_test branch November 4, 2022 15:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[release-4.10] Bug 2099843: In tests, expect that unreferenced logical router policies will not be garbage collected #1157

[release-4.10] Bug 2099843: In tests, expect that unreferenced logical router policies will not be garbage collected #1157

kyrtapz commented Jun 22, 2022 •

edited

openshift-ci bot commented Jun 22, 2022

flavio-fernandes left a comment

flavio-fernandes commented Jun 22, 2022

openshift-bot commented Jun 23, 2022

openshift-ci bot commented Jun 23, 2022

kyrtapz commented Jun 23, 2022

flavio-fernandes left a comment

openshift-ci bot commented Jun 23, 2022

openshift-ci bot commented Jun 23, 2022

openshift-ci bot commented Jun 23, 2022

flavio-fernandes commented Jun 23, 2022

flavio-fernandes commented Jun 24, 2022

openshift-ci bot commented Jun 24, 2022

openshift-bot commented Sep 22, 2022

openshift-bot commented Oct 23, 2022

kyrtapz commented Nov 3, 2022

flavio-fernandes commented Nov 3, 2022

openshift-ci bot commented Nov 4, 2022

kyrtapz commented Nov 4, 2022 •

edited

openshift-ci bot commented Nov 4, 2022

openshift-ci bot commented Nov 4, 2022

	// if no next-hops remain in the policy, remove it alltogether
	router.Policies = append(router.Policies, lrp.UUID)
	opModel := operationModel{
	Model: lrp,
	BulkOp: false,
	ErrNotFound: false,
	}
	opModels = append(opModels, opModel)

[release-4.10] Bug 2099843: In tests, expect that unreferenced logical router policies will not be garbage collected #1157

[release-4.10] Bug 2099843: In tests, expect that unreferenced logical router policies will not be garbage collected #1157

Conversation

kyrtapz commented Jun 22, 2022 • edited

openshift-ci bot commented Jun 22, 2022

flavio-fernandes left a comment

Choose a reason for hiding this comment

flavio-fernandes commented Jun 22, 2022

openshift-bot commented Jun 23, 2022

openshift-ci bot commented Jun 23, 2022

kyrtapz commented Jun 23, 2022

flavio-fernandes left a comment

Choose a reason for hiding this comment

openshift-ci bot commented Jun 23, 2022

openshift-ci bot commented Jun 23, 2022

openshift-ci bot commented Jun 23, 2022

flavio-fernandes commented Jun 23, 2022

flavio-fernandes commented Jun 24, 2022

openshift-ci bot commented Jun 24, 2022

openshift-bot commented Sep 22, 2022

openshift-bot commented Oct 23, 2022

kyrtapz commented Nov 3, 2022

flavio-fernandes commented Nov 3, 2022

openshift-ci bot commented Nov 4, 2022

kyrtapz commented Nov 4, 2022 • edited

openshift-ci bot commented Nov 4, 2022

openshift-ci bot commented Nov 4, 2022

kyrtapz commented Jun 22, 2022 •

edited

kyrtapz commented Nov 4, 2022 •

edited