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.

Sign up for GitHub

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

Jump to bottom

operator/network: add EnableUnidling flag. #357

Merged
merged 1 commit into from Jul 18, 2019
Merged

operator/network: add EnableUnidling flag. #357

merged 1 commit into from Jul 18, 2019

Conversation

squeed
Copy link
Contributor

@squeed squeed commented Jun 28, 2019

This is an openshift-sdn only feature.

Implementation PR: openshift/cluster-network-operator#214

@openshift-ci-robot openshift-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jun 28, 2019
@squeed
Copy link
Contributor Author

squeed commented Jun 28, 2019

cc @danwinship

@danwinship
Copy link
Contributor

This is an openshift-sdn only feature.

Hm... that's not totally true; it will eventually be an ovn-kubernetes feature too. But we don't expect anyone else to support it, so I guess it makes more sense as both an openshift-sdn-specific feature and an identical ovn-kubernetes-specific feature, rather than being a "generic" feature that no one except us impements. OK.

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 28, 2019
@squeed
Copy link
Contributor Author

squeed commented Jul 15, 2019

/assign @jwforres

This is ready for final api review. It's already been reviewed by an SDN team member.

@squeed
Copy link
Contributor Author

squeed commented Jul 16, 2019

hey @knobunc can I get an approved?

@squeed
Copy link
Contributor Author

squeed commented Jul 18, 2019

@knobunc @sttts @deads2k somebody anybody please merge this. it's been lgtm for weeks.

sttts
sttts reviewed Jul 18, 2019
View reviewed changes
operator/v1/types_network.go Outdated
@@ -139,6 +139,10 @@ type OpenShiftSDNConfig struct {
// it will be provided separately. If set, you must provide it yourself.
// +optional
UseExternalOpenvswitch *bool `json:"useExternalOpenvswitch,omitempty"`

// EnableUnidling controls whether or not the service proxy will support idling
Copy link
Contributor

Choose a reason for hiding this comment

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

lower case in the comment

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

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jul 18, 2019
@squeed
Copy link
Contributor Author

squeed commented Jul 18, 2019

@sttts fixed, thanks!

@sttts
Copy link
Contributor

sttts commented Jul 18, 2019

/lgtm
/approve

@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: danwinship, squeed, sttts

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-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 18, 2019
@openshift-merge-robot openshift-merge-robot merged commit 3fb3348 into openshift:master Jul 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sttts sttts sttts left review comments

@abhinavdahiya abhinavdahiya Awaiting requested review from abhinavdahiya

@adambkaplan adambkaplan Awaiting requested review from adambkaplan

Assignees

@danwinship danwinship

@sttts sttts

@jwforres jwforres

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. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@squeed @danwinship @sttts @openshift-ci-robot @jwforres @openshift-merge-robot