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

Net 6535- GatewayClassConfig controller stubs #3253

Merged
merged 6 commits into from
Nov 28, 2023

Conversation

sarahalsmiller
Copy link
Member

@sarahalsmiller sarahalsmiller commented Nov 21, 2023

Changes proposed in this PR:

  • Added stub work and registration to

How I've tested this PR:

How I expect reviewers to test this PR:

Checklist:

@sarahalsmiller sarahalsmiller added pr/no-changelog PR does not need a corresponding .changelog entry pr/no-backport signals that a PR will not contain a backport label labels Nov 22, 2023
singular: gatewayclassconfig
scope: Cluster
versions:
- name: v1alpha1
Copy link
Member

Choose a reason for hiding this comment

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

should this be v2beta1?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is the V1 gatewayClassConfig. It won't be named external like we have for the Gateway and GatewayClass, so to avoid naming issues the file is being renamed here.

items:
type: string
type: array
type: object
deployment:
description: Deployment defines the deployment configuration for the
Copy link
Contributor

Choose a reason for hiding this comment

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

We expect the GatewayClassConfig to change pretty quickly (want to rework it to something more generic), so the missing descriptions are fine.

@@ -101,6 +101,7 @@ rules:
- meshgateways
- tcproutes
- proxyconfigurations
- gatewayclassconfigs
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we move this up to be in alphabetical order?

@@ -183,7 +183,17 @@ func (c *Command) configureV2Controllers(ctx context.Context, mgr manager.Manage
Log: ctrl.Log.WithName("controller").WithName(common.MeshGateway),
Scheme: mgr.GetScheme(),
}).SetupWithManager(mgr); err != nil {
setupLog.Error(err, "unable to create controller", "controller", common.MeshGateway)
setupLog.Error(err, "unable to create controller", "controller", common.GatewayClassConfig)
Copy link
Contributor

Choose a reason for hiding this comment

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

Accidental find replace

Copy link
Contributor

@missylbytes missylbytes left a comment

Choose a reason for hiding this comment

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

Fix accidental find replace

@sarahalsmiller sarahalsmiller merged commit c3ab09e into main Nov 28, 2023
46 of 48 checks passed
@sarahalsmiller sarahalsmiller deleted the NET-6535-stub-k8s-controller branch November 28, 2023 20:12
jm96441n pushed a commit that referenced this pull request Nov 29, 2023
* rename v1 crd

* gatewayclass controller stub

* register controller

* old version

* reorder cluster role

* fix accidental find and replace error
sarahalsmiller added a commit that referenced this pull request Jan 5, 2024
* rename v1 crd

* gatewayclass controller stub

* register controller

* old version

* reorder cluster role

* fix accidental find and replace error
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/no-backport signals that a PR will not contain a backport label pr/no-changelog PR does not need a corresponding .changelog entry theme/mesh-gw
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants