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.

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

CFE-617: Rebase v2.4.4 #11

Merged
merged 89 commits into from
Nov 15, 2022
Merged

Conversation

thejasn
Copy link

@thejasn thejasn commented Oct 13, 2022

Description

Rebase controller to v2.4.4. Steps followed.

  • git checkout -b rebase-2.4.4 v2.4.4
  • git merge -s ours openshift/main
  • Compared diff before and after merge using git diff openshift/main v2.4.4 [Diff is the same]

Note:

kishorj and others added 30 commits March 18, 2022 15:49
The example ingress does not use any annotations.
* fix formatting

* another formatting fix
* Misc minor fixes to docs

* more fixes
* Auto generate CRDs for the Helm chart

Co-authored-by: Fawad Khaliq <fawadkh@amazon.com>

* Makefile changes

* fix kustomize install

Co-authored-by: Fawad Khaliq <fawadkh@amazon.com>
…ubernetes-sigs#2575)

* adding support for topologySpreadConstraints

* update readme
…empty map (kubernetes-sigs#2576)

* adding support to set affinity in podSpec to the empty map

* remove typo

* updated readme

Co-authored-by: Kishor Joshi <joshikis@amazon.com>
…espace

check namespace when update pod condition for deleted TGB
Signed-off-by: thejasn <thn@redhat.com>
* sort ingress rules by path length

* add unit test

* sort paths instead of sorting rules directly

* update sorting strategy
Updated deployment for prometheus.io annotations when sericeMonitor i…
…-only-ingress

Added feature gate to restrict to only ingress resources
Format `clusterSecretsPermissions.allowAllSecrets` as the setting was bit hidden in the text.
Also see - awsdocs/amazon-eks-user-guide#553
Signed-off-by: krrrr38 <k.kaizu38@gmail.com>
* fix log level

* Update pkg/backend/endpoint_resolver.go

Co-authored-by: Kishor Joshi <joshikis@amazon.com>
* feat: add optional service monitor namespace

* namespace to None
…es-sigs#2624)

* Add option for imagePullSecrets on service account

* Make default imagePullSecrets empty

* Update helm/aws-load-balancer-controller/values.yaml

* Update helm/aws-load-balancer-controller/values.yaml

Co-authored-by: Kishor Joshi <joshikis@amazon.com>
This commit relaxes the restrictions around the `group.order`
annotation, allowing both negative and duplicate orders. Duplicate
orders are sorted lexicographically.
@openshift-ci
Copy link

openshift-ci bot commented Oct 27, 2022

@thejasn: all tests passed!

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.

@Miciah
Copy link

Miciah commented Nov 2, 2022

/assign

@candita
Copy link

candita commented Nov 2, 2022

/assign @gcs278

@thejasn thejasn changed the title [WIP] CFE-617: Rebase v2.4.4 CFE-617: Rebase v2.4.4 Nov 3, 2022
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 3, 2022
@gcs278
Copy link

gcs278 commented Nov 3, 2022

Should we disable the IP mode since this feature gate is available upstream?

I'm a bit confused by this, maybe I'm misunderstanding. We pull an upstream commit to enable this feature-gate (which defaults to true) and now the rebase pulls in that feature gate (which defaults to true). Why would we need to do anything differently other than just dropping the carry now that it's in our rebase?

@gcs278
Copy link

gcs278 commented Nov 3, 2022

Should we disable the reconciliation of the services since they are not needed for the moment?

Does it do any harm to leave it enabled?

@thejasn
Copy link
Author

thejasn commented Nov 4, 2022

Should we disable the IP mode since this feature gate is available upstream?

I'm a bit confused by this, maybe I'm misunderstanding. We pull an upstream commit to enable this feature-gate (which defaults to true) and now the rebase pulls in that feature gate (which defaults to true). Why would we need to do anything differently other than just dropping the carry now that it's in our rebase?

So target-type=ip is not supported on Openshift since it requires AWS VPC CNI and prior to the upstream patch which added EnableIPTargetType feature gate there was no error on the controller when target-type was set to ip on Openshift. So the feature gate allows us to disable this target-type which in turns gives an explicit error Unsupported target-type=ip.
The follow up PR on the operator is openshift/aws-load-balancer-operator#80

@thejasn
Copy link
Author

thejasn commented Nov 4, 2022

Should we disable the reconciliation of the services since they are not needed for the moment?

Does it do any harm to leave it enabled?

No, we decided we will allow reconciliation of Services as is and just document that the operator will support Service type resources as well by creating NLBs for them. Docs PR for the same.

@gcs278
Copy link

gcs278 commented Nov 7, 2022

Looks good to me after reading through all of the changes, but since it's already approved, and a LGTM would merge it, @Miciah do you have any concerns or want a chance to review before I LGTM it?

@thejasn
Copy link
Author

thejasn commented Nov 7, 2022

/hold for @Miciah to review.

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 7, 2022
@gcs278
Copy link

gcs278 commented Nov 7, 2022

Thanks I forgot about lgtm and hold.
/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 7, 2022
@Miciah
Copy link

Miciah commented Nov 11, 2022

/lgtm
/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 11, 2022
@xenolinux
Copy link

/label docs-approved

@openshift-ci openshift-ci bot added the docs-approved Signifies that Docs has signed off on this PR label Nov 14, 2022
@thejasn
Copy link
Author

thejasn commented Nov 14, 2022

/assign @lihongan

@lihongan
Copy link

/label qe-approved

@openshift-ci openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Nov 15, 2022
@thejasn
Copy link
Author

thejasn commented Nov 15, 2022

/label px-approved

As per Chris Fields: the docs are enough for this release of AWS Load Balancer Operator.

@openshift-ci openshift-ci bot added the px-approved Signifies that Product Support has signed off on this PR label Nov 15, 2022
@openshift-merge-robot openshift-merge-robot merged commit 2c04703 into openshift:main Nov 15, 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.

None yet