Skip to content

Conversation

@frobware
Copy link
Contributor

@frobware frobware commented Apr 6, 2022

Adds a new field to the IngressController API to allow the maximum
number of simultaneous connections (i.e., maxconn) be configurable by
cluster administrators:

openshift/enhancements#1084

@openshift-ci openshift-ci bot requested review from adambkaplan and soltysh April 6, 2022 17:02
@frobware frobware force-pushed the haproxy-maxconn branch 2 times, most recently from b0e336e to 3cab877 Compare April 6, 2022 18:04
frobware added a commit to frobware/cluster-ingress-operator that referenced this pull request Apr 7, 2022
Switch from the unsupported config override to the tuning option
introduced in openshift/api#1161.
Copy link
Contributor

@gcs278 gcs278 left a comment

Choose a reason for hiding this comment

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

Reviewed and updates look good to me. Will let others lgtm.

@frobware frobware force-pushed the haproxy-maxconn branch 2 times, most recently from 7cc4481 to 5ed19fc Compare April 14, 2022 11:16
frobware added a commit to frobware/cluster-ingress-operator that referenced this pull request Apr 14, 2022
Switch from the unsupported config override to the tuning option
introduced in openshift/api#1161.
@deads2k
Copy link
Contributor

deads2k commented Apr 21, 2022

The API itself looks ok, but I'd like to understand how the operator will ensure the pods can function. I expected to see something like, "the operator may choose to increase memory requests to cover higher values". Not a specific promise of how or how much, but an indication that the operator will make adjustments

frobware added a commit to frobware/cluster-ingress-operator that referenced this pull request Apr 21, 2022
Switch from the unsupported config override to the tuning option
introduced in openshift/api#1161.
frobware added a commit to frobware/cluster-ingress-operator that referenced this pull request Apr 25, 2022
Switch from the unsupported config override to the tuning option
introduced in openshift/api#1161.
frobware added a commit to frobware/cluster-ingress-operator that referenced this pull request Apr 27, 2022
Switch from the unsupported config override to the tuning option
introduced in openshift/api#1161.
frobware added a commit to frobware/enhancements that referenced this pull request Apr 27, 2022
@frobware frobware force-pushed the haproxy-maxconn branch 2 times, most recently from e323cdd to 60d1090 Compare April 27, 2022 12:42
Adds a new field to the IngressController API to allow the maximum
number of simultaneous connections (i.e., maxconn) be configurable by
cluster administrators:

openshift/enhancements#1084
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Apr 28, 2022

@frobware: all tests passed!

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.

frobware added a commit to frobware/enhancements that referenced this pull request Apr 28, 2022
@deads2k
Copy link
Contributor

deads2k commented Apr 28, 2022

looks great.

/lgtm
/approve

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Apr 28, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Apr 28, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, frobware

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 Apr 28, 2022
@ahardin-rh
Copy link

/label docs-approved

@openshift-ci openshift-ci bot added the docs-approved Signifies that Docs has signed off on this PR label Apr 28, 2022
@CFields651
Copy link

/label px-approved

@openshift-ci openshift-ci bot added the px-approved Signifies that Product Support has signed off on this PR label Apr 28, 2022
@ShudiLi
Copy link
Member

ShudiLi commented Apr 29, 2022

/label qe-approved

@openshift-ci openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Apr 29, 2022
@openshift-merge-robot openshift-merge-robot merged commit 1469fac into openshift:master Apr 29, 2022
frobware added a commit to frobware/cluster-ingress-operator that referenced this pull request Apr 29, 2022
$ go get github.com/openshift/api@1469fac35c7515f007526e51f519eef7339a1bbb
$ go mod vendor
$ go mod tidy
frobware added a commit to frobware/cluster-ingress-operator that referenced this pull request Apr 29, 2022
Switch from the unsupported config override to the tuning option
introduced in openshift/api#1161.
frobware added a commit to frobware/cluster-ingress-operator that referenced this pull request Apr 29, 2022
$ go get github.com/openshift/api@1469fac35c7515f007526e51f519eef7339a1bbb
$ go mod vendor
$ go mod tidy
frobware added a commit to frobware/cluster-ingress-operator that referenced this pull request May 9, 2022
$ go get github.com/openshift/api@1469fac35c7515f007526e51f519eef7339a1bbb
$ go mod vendor
$ go mod tidy
thejasn pushed a commit to thejasn/enhancements that referenced this pull request May 10, 2022
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. docs-approved Signifies that Docs has signed off on this PR lgtm Indicates that a PR is ready to be merged. px-approved Signifies that Product Support has signed off on this PR qe-approved Signifies that QE has signed off on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants