Skip to content

Conversation

@MaysaMacedo
Copy link
Contributor

When Kuryr is used creation of Services with Load Balancer type
are restricted to when a Router and External Connectivity exists
on the Cluster.

@MaysaMacedo MaysaMacedo changed the title Add note about restrictions with svc of LB type Add note about restrictions with svc of LB type with Kuryr Sep 16, 2020
@openshift-ci-robot openshift-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Sep 16, 2020
@MaysaMacedo MaysaMacedo changed the title Add note about restrictions with svc of LB type with Kuryr Add note about restrictions svc of LB type with Kuryr Sep 16, 2020
@MaysaMacedo
Copy link
Contributor Author

/cc @luis5tb

@luis5tb
Copy link
Contributor

luis5tb commented Sep 16, 2020

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 16, 2020
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Sep 16, 2020
@openshift-ci-robot
Copy link

New changes are detected. LGTM label has been removed.

Copy link
Contributor

@maxwelldb maxwelldb left a comment

Choose a reason for hiding this comment

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

Thoughts on this edit?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, if the Machines Subnet is not connected to a Router or it’s connected, but the Router has no external gateway set, Kuryr won’t create Floating IPs for Services with LoadBalancer type.

->

[line break]
If the machines subnet is not connected to a router, or if the subnet is connected, but the router has no external gateway set, Kuryr cannot create floating IPs for load balancer services.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I modified "load balancer services" to "LoadBalancer services" to give a better indication that it's the type of the Service.

Copy link
Contributor

@maxwelldb maxwelldb Sep 21, 2020

Choose a reason for hiding this comment

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

[line break]
If the machines subnet is not connected to a router, or if the subnet is connected, but the router has no external gateway set, Kuryr cannot create floating IPs for LoadBalancer services.

That, then? @MaysaMacedo

Copy link
Contributor

Choose a reason for hiding this comment

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

Committing directly.

@maxwelldb
Copy link
Contributor

Which versions should this apply to?

@MaysaMacedo
Copy link
Contributor Author

Which versions should this apply to?

4.6

When Kuryr is used creation of Services with Load Balancer type
are restricted to when a Router and External Connectivity exists
on the Cluster.
@maxwelldb
Copy link
Contributor

@MaysaMacedo
Copy link
Contributor Author

MaysaMacedo commented Sep 21, 2020

Looks good @maxwelldb . Thanks!

@maxwelldb maxwelldb added the peer-review-needed Signifies that the peer review team needs to review this PR label Sep 21, 2020
@maxwelldb
Copy link
Contributor

@MaysaMacedo TY! Do you have anyone from QE who's familiar with this and could +1 it?

@maxwelldb maxwelldb self-requested a review September 21, 2020 13:50
@eurijon
Copy link

eurijon commented Sep 21, 2020

It looks good from QE side!

@MaysaMacedo
Copy link
Contributor Author

It looks good from QE side!

Thanks, Jon.

@ahardin-rh ahardin-rh added peer-review-done Signifies that the peer review team has reviewed this PR and removed peer-review-needed Signifies that the peer review team needs to review this PR labels Sep 21, 2020
@ahardin-rh ahardin-rh added this to the Future Release milestone Sep 21, 2020
@ahardin-rh
Copy link
Contributor

LGTM! You just need to squash the two commits down into one. Thanks!

@maxwelldb
Copy link
Contributor

Thanks!

@maxwelldb maxwelldb merged commit c4219f1 into openshift:master Sep 21, 2020
@maxwelldb
Copy link
Contributor

/cherry-pick enterprise-4.6

@openshift-cherrypick-robot

@maxwelldb: new pull request created: #25623

Details

In response to this:

/cherry-pick enterprise-4.6

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

branch/enterprise-4.6 peer-review-done Signifies that the peer review team has reviewed this PR size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants