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

WIP Ingress-gce helm chart #852

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

kron4eg
Copy link

@kron4eg kron4eg commented Sep 30, 2024

How to categorize this PR?

/area control-plane
/area networking
/kind enhancement
/platform gcp

What this PR does / why we need it:

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Release note:


@gardener-robot gardener-robot added area/control-plane Control plane related area/networking Networking related kind/enhancement Enhancement, improvement, extension platform/gcp Google cloud platform/infrastructure labels Sep 30, 2024
@gardener-robot
Copy link

@kron4eg Thank you for your contribution.

@gardener-robot gardener-robot added needs/review Needs review size/l Size of pull request is large (see gardener-robot robot/bots/size.py) needs/second-opinion Needs second review by someone else labels Sep 30, 2024
@gardener-robot-ci-2
Copy link
Contributor

Thank you @kron4eg for your contribution. Before I can start building your PR, a member of the organization must set the required label(s) {'reviewed/ok-to-test'}. Once started, you can check the build status in the PR checks section below.

verbs: ["get", "list", "watch"]
# For now, GLBC annotates ingress resources with various state and statuses:
# https://github.com/kubernetes/ingress-gce/blob/50d49b077d9ab4362a02fae05f94e433cd3f08dc/pkg/controller/controller.go#L579
# TODO(rramkumar1): Remove unnecessary `update` permission once statuses are propagated through `ingresses/status`
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need to include the upstream TODOs. Same in l.70

@@ -5,3 +5,5 @@ cloud-controller-manager:
enabled: true
csi-driver-controller:
enabled: true
ingress-gce:
enabled: true
Copy link
Member

Choose a reason for hiding this comment

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

From what I saw during testing/playing around with this controller, I'd rather not have it enabled by default. In my opinion that should be an option, unless there are cases where it needs to be deployed.

@ScheererJ ScheererJ mentioned this pull request Oct 10, 2024
53 tasks
@gardener-robot gardener-robot added kind/api-change API change with impact on API users size/xl Size of pull request is huge (see gardener-robot robot/bots/size.py) and removed size/l Size of pull request is large (see gardener-robot robot/bots/size.py) labels Oct 31, 2024
@kron4eg kron4eg force-pushed the ingress-gce-chart branch 2 times, most recently from 1bea128 to 71f3654 Compare November 1, 2024 17:31
@kron4eg kron4eg changed the title Ingress-gce helm chart WIP Ingress-gce helm chart Nov 4, 2024
@gardener-robot gardener-robot added size/l Size of pull request is large (see gardener-robot robot/bots/size.py) and removed size/xl Size of pull request is huge (see gardener-robot robot/bots/size.py) labels Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/control-plane Control plane related area/networking Networking related kind/api-change API change with impact on API users kind/enhancement Enhancement, improvement, extension needs/review Needs review needs/second-opinion Needs second review by someone else platform/gcp Google cloud platform/infrastructure size/l Size of pull request is large (see gardener-robot robot/bots/size.py)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants