-
Notifications
You must be signed in to change notification settings - Fork 118
Use new types to process resource changes #124
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Just a few minor comments/questions
if gw.Name == impl.ControllerName() { | ||
impl.Logger().Info("Found correct Gateway resource", | ||
if gw.Namespace != impl.conf.GatewayNsName.Namespace || gw.Name != impl.conf.GatewayNsName.Name { | ||
impl.Logger().Info("Gateway was upserted but ignored based on its namespace/name", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should add the expected ns name to this log so the user has the ability to fix the error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this always will be an error though. Because let's say the admin deploys multiple NGINX Gateways, each watching its own Gateway resources.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True. Once we support configurable gateway names we can change this back.
a3df36f
to
107c3e3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
This commit plugs in the new types introduced in 0a39e3f to process changes to resources and generate NGINX configuration.
107c3e3
to
6ba113d
Compare
Proposed changes
This PR plugs in the new types introduced in #121 to process
changes to resources and generate NGINX configuration.
The installation manifests and examples were also updated to support the Gateway resource.
Note: the status updating was removed. In will be reintroduced in the next PR -- an updated version what works with the new types.