Skip to content

Conversation

@qinguoyi
Copy link
Member

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

.PHONY: helm-install
helm-install: helm
helm upgrade --install llmaz ./chart --namespace llmaz-system --create-namespace -f ./chart/values.global.yaml

### Install
```cmd
helm repo add inftyai https://inftyai.github.io/llmaz
helm repo update
helm install llmaz inftyai/llmaz --namespace llmaz-system --create-namespace --version 0.0.3
```

### Uninstall
```cmd
helm uninstall llmaz
kubectl delete ns llmaz-system
```

# Adds namespace to all resources.
namespace: llmaz-system

spec:
conversion:
strategy: Webhook
webhook:
clientConfig:
service:
name: llmaz-webhook-service
namespace: llmaz-system
path: /convert
conversionReviewVersions:
- v1

what can i do for these?

Does this PR introduce a user-facing change?

NONE

@InftyAI-Agent InftyAI-Agent added needs-triage Indicates an issue or PR lacks a label and requires one. needs-priority Indicates a PR lacks a label and requires one. do-not-merge/needs-kind Indicates a PR lacks a label and requires one. labels Sep 24, 2024
@kerthcet
Copy link
Member

Thanks @qinguoyi for the help, let me take a try first.

| nindent 10 }}
securityContext:
runAsNonRoot: true
securityContext: {{- toYaml .Values.controllerManager.podSecurityContext | nindent
Copy link
Member

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.

Copy link
Member Author

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.")
Copy link
Member

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.

Copy link
Member Author

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
Copy link
Member

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.

Copy link
Member Author

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.

@qinguoyi
Copy link
Member Author

as suggested, i re-push the code. @kerthcet

@kerthcet
Copy link
Member

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
/approve
/kind feature

@InftyAI-Agent InftyAI-Agent added lgtm Looks good to me, indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. feature Categorizes issue or PR as related to a new feature. and removed do-not-merge/needs-kind Indicates a PR lacks a label and requires one. labels Sep 24, 2024
@InftyAI-Agent InftyAI-Agent merged commit ab43f9c into InftyAI:main Sep 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. feature Categorizes issue or PR as related to a new feature. lgtm Looks good to me, indicates that a PR is ready to be merged. needs-priority Indicates a PR lacks a label and requires one. needs-triage Indicates an issue or PR lacks a label and requires one.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support install llmaz at any namespace

3 participants