-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat(agent): add vcluster crd and controller #259
Conversation
@@ -1,4 +1,4 @@ | |||
FROM golang:1.22.4-alpine3.20 as builder | |||
FROM golang:1.22.6-alpine3.20 AS builder |
Check notice
Code scanning / Trivy
No HEALTHCHECK defined Low
Type: dockerfile
Vulnerability DS026
Severity: LOW
Message: Add HEALTHCHECK instruction in your Dockerfile
Link: DS026
how do you customize the helm values for installing our agent in the cluster w/ this structure? i think a single crd to install vcluster + agent is not a bad idea (my original thought was you'd just use a service deployment for the vcluster though), but we also need to be able to specify agent helm values and helm repository |
Oh wait there's helm values for vcluster plus the agent, that's nice |
return *in.RepoUrl | ||
} | ||
|
||
type VClusterHelmConfiguration struct { |
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 there a reason why you inline the helm configuration struct for vcluster and agent? Suppose it's nice for the types to self-document
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.
That's just not to add an additional level to access the structure. Initially I have used HelmConfiguration directly, but I wanted to have some methods that can default values required to initialize cluster/agent such as repo url or chart name. This way structure access looks the same from the outside but there are different method implementations.
I think we should still want to support separately created vclusters. So if we could add a bool field to the spec specifying maybe |
Ok, so when external flag is set then we only upsert to the API, install the agent using provided kubeconfig ref and skip the vcluster chart installation? |
Yeah precisely |
template
package to be able to reuse added repositoriesCRD
CRD (for testing locally)