Skip to content

Comments

Bug 1954773: adding cli flag for egressfirewall and removing CRD watcher#541

Merged
openshift-merge-robot merged 3 commits intoopenshift:masterfrom
JacobTanenbaum:BZ1954773
May 26, 2021
Merged

Bug 1954773: adding cli flag for egressfirewall and removing CRD watcher#541
openshift-merge-robot merged 3 commits intoopenshift:masterfrom
JacobTanenbaum:BZ1954773

Conversation

@JacobTanenbaum
Copy link
Contributor

cherry-picked three commits from upstream ovn-kube to apply cleanly and solve the problem

c37c866f78765dfe15fb06943ead64d0723fedba - change the egressFirewall to use a cli flag instead of dynamically turn
on from watching CRDs

2e870eb23259c71216bb56895d63394d84166527 - Remove node-side egress IP clients to help everything apply cleanly

f9af29bbd67dda81d7091c9651cef0cb3b1ff895 - remove the CRD watch and related bits as they are no longer needed

- What this PR does and why is it needed

- Special notes for reviewers

- How to verify it

- Description for the changelog

JacobTanenbaum and others added 3 commits May 14, 2021 12:36
on from watching CRDs

Signed-off-by: Jacob Tanenbaum <jtanenba@redhat.com>
Signed-off-by: Alexander Constantinescu <aconstan@redhat.com>
since moving egressFirewall to a cli flag there is no longer any need to
watch CRDs

Signed-off-by: Jacob Tanenbaum <jtanenba@redhat.com>
@openshift-ci openshift-ci bot added the bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. label May 14, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 14, 2021

@JacobTanenbaum: This pull request references Bugzilla bug 1954773, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.8.0) matches configured target release for branch (4.8.0)
  • bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

No GitHub users were found matching the public email listed for the QA contact in Bugzilla (anusaxen@redhat.com), skipping review request.

Details

In response to this:

Bug 1954773: adding cli flag for egressfirewall and removing CRD watcher

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 openshift-ci bot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label May 14, 2021
@openshift-ci openshift-ci bot requested review from dcbw and trozet May 14, 2021 16:53
@JacobTanenbaum
Copy link
Contributor Author

/retest

1 similar comment
@JacobTanenbaum
Copy link
Contributor Author

/retest

Copy link
Contributor

@alexanderConstantinescu alexanderConstantinescu left a comment

Choose a reason for hiding this comment

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

This is more of a question, but why do we need commit: Remove node-side egress IP clients for this patch?

Copy link
Contributor

@alexanderConstantinescu alexanderConstantinescu left a comment

Choose a reason for hiding this comment

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

This is more of a question, but why do we need commit: Remove node-side egress IP clients for this patch?

Actually, never mind I can answer my own question: "because remove the CRD watch and related bits as they are no longer needed also touched pkg/node/ovn_test.go which means we need that commit to cleanly cherry-pick"

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label May 17, 2021
@JacobTanenbaum
Copy link
Contributor Author

/restest

@JacobTanenbaum
Copy link
Contributor Author

/retest

3 similar comments
@JacobTanenbaum
Copy link
Contributor Author

/retest

@JacobTanenbaum
Copy link
Contributor Author

/retest

@JacobTanenbaum
Copy link
Contributor Author

/retest

@knobunc
Copy link
Contributor

knobunc commented May 21, 2021

/approve
/retest

@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 21, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alexanderConstantinescu, JacobTanenbaum, knobunc

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

The pull request process is described here

Details 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 21, 2021
@knobunc
Copy link
Contributor

knobunc commented May 21, 2021

@JacobTanenbaum can you see if you can work out what is wrong with the Windows test? It seems to fail for everyone. Perhaps Jamo has some insight.

@knobunc
Copy link
Contributor

knobunc commented May 21, 2021

/override ci/prow/e2e-aws-ovn-windows

@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 21, 2021

@knobunc: Overrode contexts on behalf of knobunc: ci/prow/e2e-aws-ovn-windows

Details

In response to this:

/override ci/prow/e2e-aws-ovn-windows

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-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

1 similar comment
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@knobunc
Copy link
Contributor

knobunc commented May 21, 2021

/retest

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

1 similar comment
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

19 similar comments
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@knobunc
Copy link
Contributor

knobunc commented May 26, 2021

/override ci/prow/e2e-aws-ovn-windows
/override ci/prow/e2e-metal-ipi-ovn-dualstack

There is another PR open to fix the test failures.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 26, 2021

@knobunc: Overrode contexts on behalf of knobunc: ci/prow/e2e-aws-ovn-windows, ci/prow/e2e-metal-ipi-ovn-dualstack

Details

In response to this:

/override ci/prow/e2e-aws-ovn-windows
/override ci/prow/e2e-metal-ipi-ovn-dualstack

There is another PR open to fix the test failures.

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-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@JacobTanenbaum
Copy link
Contributor Author

/retest

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

1 similar comment
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 26, 2021

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

Test name Commit Details Rerun command
ci/prow/4.8-upgrade-from-stable-4.7-e2e-aws-ovn-upgrade fc18695 link /test 4.8-upgrade-from-stable-4.7-e2e-aws-ovn-upgrade

Full PR test history. Your PR dashboard.

Details

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
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit 1a73966 into openshift:master May 26, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 26, 2021

@JacobTanenbaum: Some pull requests linked via external trackers have merged:

The following pull requests linked via external trackers have not merged:

These pull request must merge or be unlinked from the Bugzilla bug in order for it to move to the next state. Once unlinked, request a bug refresh with /bugzilla refresh.

Bugzilla bug 1954773 has not been moved to the MODIFIED state.

Details

In response to this:

Bug 1954773: adding cli flag for egressfirewall and removing CRD watcher

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.

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. bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants