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

Ensure uniqueness and validity of generated names and labels #716

Merged
merged 3 commits into from
Jun 6, 2023

Conversation

pleshakov
Copy link
Contributor

@pleshakov pleshakov commented Jun 6, 2023

Problem:

Provisioner could generate an invalid name or a label for NKG static mode deployment. As a result, the provisioner would fail with an error like below:

{“level”:“info”,“ts”:“2023-06-05T17:26:04Z”,“logger”:“eventLoop”,“msg”:“added an event to the next batch”,“type”:“*events.UpsertEvent”,“total”:1}
panic: failed to create deployment: Deployment.apps “nginx-gateway-gateway-conformance-infra-same-namespace-with-http-listener-on-8080" is invalid: [spec.selector.matchLabels: Invalid value: “nginx-gateway-gateway-conformance-infra-same-namespace-with-http-listener-on-8080”: must be no more than 63 characters, spec.selector: Invalid value: v1.LabelSelector{MatchLabels:map[string]string{“app”:“nginx-gateway-gateway-conformance-infra-same-namespace-with-http-listener-on-8080”}, MatchExpressions:[]v1.LabelSelectorRequirement(nil)}: invalid label selector]

Solution:

Provisioner will use the following format for names and labels of Deployments:
nginx-gateway-<number>, where the number if an interger >= 1. For every new Gateway resource, the provisioner will increment the number by one.

This approach will break if the provisioner is restarted, but we don't support provisioner for production yet, so it is acceptable.

Note: correlation between a Gateway resource and the corresponding Deployment can be found in the provisioner log. For example:

{"level":"info","ts":"2023-06-05T21:23:33Z","logger":"eventHandler","msg":"Created deployment","deployment":{"name":"nginx-gateway-1","namespace":"nginx-gateway"},"gateway":{"name":"same-namespace","namespace":"gateway-conformance-infra"}}

This PR will unblock #713 (comment)

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

Problem:

Provisioner could generate an invalid name or a label for NKG static
mode deployment. As a result, the provisioner would fail with an error
like below:
```
{“level”:“info”,“ts”:“2023-06-05T17:26:04Z”,“logger”:“eventLoop”,“msg”:“added an event to the next batch”,“type”:“*events.UpsertEvent”,“total”:1}
panic: failed to create deployment: Deployment.apps “nginx-gateway-gateway-conformance-infra-same-namespace-with-http-listener-on-8080" is invalid: [spec.selector.matchLabels: Invalid value: “nginx-gateway-gateway-conformance-infra-same-namespace-with-http-listener-on-8080”: must be no more than 63 characters, spec.selector: Invalid value: v1.LabelSelector{MatchLabels:map[string]string{“app”:“nginx-gateway-gateway-conformance-infra-same-namespace-with-http-listener-on-8080”}, MatchExpressions:[]v1.LabelSelectorRequirement(nil)}: invalid label selector]
```


Solution:

Provisioner will use the following format for names and labels of
Deployments:
nginx-gateway-<number>, where the number if an interger >= 1. For every
new Gateway resource, the provisioner will increment the number by one.

This approach will break if the provisioner is restarted, but we don't
support provisioner for production yet, so it is acceptable.


Note: correlation between a Gateway resource and the corresponding
Deployment can be found in the provisioner log. For example:
```
{"level":"info","ts":"2023-06-05T21:23:33Z","logger":"eventHandler","msg":"Created deployment","deployment":{"name":"nginx-gateway-1","namespace":"nginx-gateway"},"gateway":{"name":"same-namespace","namespace":"gateway-conformance-infra"}}
```
@pleshakov pleshakov requested a review from a team as a code owner June 6, 2023 00:42
@github-actions github-actions bot added the bug Something isn't working label Jun 6, 2023
@pleshakov pleshakov requested review from kate-osborn and sjberman June 6, 2023 17:35
@pleshakov pleshakov merged commit f188b4c into nginxinc:main Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants