-
-
Notifications
You must be signed in to change notification settings - Fork 44
feat:support apply llmaz to any ns #172
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
|
Thanks @qinguoyi for the help, let me take a try first. |
chart/templates/deployment.yaml
Outdated
| | nindent 10 }} | ||
| securityContext: | ||
| runAsNonRoot: true | ||
| securityContext: {{- toYaml .Values.controllerManager.podSecurityContext | nindent |
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.
Is this change related? If not, we can have another PR to cover your case.
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.
when i execute the cmd "make helm", this is auto generated. i revert this
cmd/main.go
Outdated
| var probeAddr string | ||
| flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.") | ||
| flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.") | ||
| flag.StringVar(&secretNamespace, "webhook-secret-namespace", "llmaz-system", "The namespace of the secret that contains the webhook server's TLS certificate and key.") |
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.
Let's rename this to namespace intuitively. Same to other places, secret is just one of the resources in the namespace I think.
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.
ok, i update this soon.
| func main() { | ||
| var metricsAddr string | ||
| var enableLeaderElection bool | ||
| var probeAddr string |
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.
Please don't, let's make sure this variables only available in main function for now.
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.
ok ,i revert this soon.
|
as suggested, i re-push the code. @kerthcet |
|
Thanks @qinguoyi this makes sense to me. But we have some confusions on the document right now because we have two different ways to install lws, one with kustomize, another with helm chart, I'll update the document later to make sure people can participate in the project development easier. Thanks anyway. /lgtm |
What this PR does / why we need it
Support install llmaz at any namespace
Which issue(s) this PR fixes
Fixes #141
Special notes for your reviewer
i update the cert ns from hardcode to chart configuration, but there also has many "llamz-system" hardcode, such as :
llmaz/Makefile
Lines 300 to 303 in 4c35e01
llmaz/docs/installation.md
Lines 10 to 16 in 4c35e01
llmaz/docs/installation.md
Lines 44 to 49 in 4c35e01
llmaz/config/default/kustomization.yaml
Lines 1 to 3 in 4c35e01
llmaz/chart/crds/openmodel-crd.yaml
Lines 7 to 17 in 4c35e01
what can i do for these?
Does this PR introduce a user-facing change?
NONE