-
Notifications
You must be signed in to change notification settings - Fork 9
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
[DO NOT MERGE] First swing at minimizing README. #123
base: develop
Are you sure you want to change the base?
Conversation
* CP-22731: include cz-insights-controller as subchart * increase replicacount for tag server * CP-22731: add beta testing * update release process for insights controller * update release workflow * make most resources off by default * update readme * use global for secret names * incorporate changes from 0.0.30-beta * add beta release doc * use local chart for testing --------- Co-authored-by: josephbarnett <[email protected]>
* CP-23425: set default max retries * update init job to work with long running scrapes * increase wait time for scrape endpoint * default batch size added * increase wait time for init job * adjust remote write threshold, add default resource values
* CP-23051: Change default kube-state-metrics behavior to use Cloudzero subchart (#91) * override KSM name * enable ksm by default * CP-23388: Define Static KubeStateMetrics Target Endpoint (#99) * add 1.0.2 release doc file --------- Co-authored-by: bdrennz <[email protected]>
#116) * remove unused metric * add kubemetrics * bump chart version for beta * use dev tag for validator * fix endpoint var name * allow github to bump version * simplify metric logic * update tag * use dev tag for chart
* insights-controller added to agent chart
* CP-23428: add certificate helm chart * update with documentation comments * Update charts/cloudzero-agent/README.md Co-authored-by: Becki Lee <[email protected]> * Update charts/cloudzero-agent/README.md remove duplicate entry Co-authored-by: Becki Lee <[email protected]> * Update charts/cloudzero-agent/README.md add period to end of sentence in readme Co-authored-by: Becki Lee <[email protected]> * PR suggestion for readme * update config example --------- Co-authored-by: Becki Lee <[email protected]>
CP-24028: add scrape target for insights container CP-22734: Bump insights image release version Enhance README for helm repo management Add release note for next beta version Update release process for customer version numbers in betas
* CP-23892, CP-24009, CP-23959: release note * add healthcheck support * bump value of insights controller
charts/cloudzero-agent/README.md
Outdated
|
||
To use the chart or a beta version, you must add the repository to Helm. Refer to the [`helm repo`](https://helm.sh/docs/helm/helm_repo/) documentation for command details. | ||
|
||
#### 1. Add the Helm Chart Repository |
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.
Looking to reduce complexity here. Here are two commands to get the agent spun with some sensible defaults.
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'm all in favor of having an "easy install" option. but, I feel that one of our issues so far is that "sensible defaults" varies a lot. for example, in previous versions, no labels were enabled by default. so customers would install, and then complain later their labels were missing, and we found that users weren't digging into the docs too much to see that labels had to be manually enabled.
in the recent beta, our solution to this was to force users to make choices at install time, so that a user needs to specify if they want labels or not, which label they want, etc. so, installation might be a bit harder, but the user would be forced to make the (hopefully) correct choices.
but all of that is to say, I am all for trying to focus in on a sensible default setup
charts/cloudzero-agent/README.md
Outdated
|
||
**Cert Manager** |
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.
Do we need to ship subcharts? I would suggest going ahead and highly tuning the prometheus and KSM image we want to ship so we're not trying to pass details or manage subcharts. IE, a customer doesn't necessarily care that we are shipping a subchart provided by prometheus, they're still going to hold us accountable for the image that we're shipping.
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.
yeah, I'd be happy to not use subcharts. for the two that we currently use:
- KSM - pulling it into our chart should be easy (we have an old ticket for it here)
- cert-manager - I think not using cert-manager would be great given that customers have had issues with it. but, while it can be difficult at install time, I think it's probably the easiest way to manage the cert afterwards. a cert is required if the customer wants to export labels or annotations, because we get labels/annotations from a validating webhook configuration, which requires a tls cert. the options I explored for that are:
a. cert-manager - works well but can be hard to set up, creates a self-signed cert by default, but can be configured as desired. rotates cert automatically
b. cloudzero-certificate helm chat - I added this chart to the beta release to create a standalone self-signed cert. the reason I didn't just add this to the cloudzero-agent chart is that the helm functions used for generating the cert won't work well with things like argo, since argo will see the change and continually try to sync.
c. users can also provide a tls cert via a secret, or by adding annotations and whatnot to the insightsController deployment if they're using some secret injection setup. described here. this is clunky to configure.
another option that we could explore is to just build our own little version of cert-manager and include it in the chart. that would solve all our problems - easy install, but would also manage certs over time.
anyway, sorry for the essay. tldr, I'd like to not use cert-manager, but then we need another way to create and manage tls certs
charts/cloudzero-agent/README.md
Outdated
helm repo update | ||
``` | ||
|
||
2. Provision a TLS certificate; by default, this chart deploys a `ValidatingWebhookConfiguration` resource, which requires a certificate in order validate requests to the webhook server. See related Kubernetes documentation [here](https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#configure-admission-webhooks-on-the-fly). |
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.
Would leave out this entirely so we don't cause confusion, but document thoroughly in the values file to ensure we give folks the necessary customization for security teams.
charts/cloudzero-agent/README.md
Outdated
```yaml | ||
# -- Account ID of the account the cluster is running in. This must be a string - even if it is a number in your system. |
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.
How could we get this down to 1 single key for the customer instead of asking for region, accountid, and keyid? Ideally we just ask for a singular api 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.
we have a ticket for this here https://cloudzero.atlassian.net/browse/CP-23666
helm install <RELEASE_NAME> cloudzero/cloudzero-agent -f configuration.example.yaml | ||
``` | ||
|
||
### Mandatory Values |
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 move this to the values file and use that as the source of truth...also, let's adjust to where all but only the necessary have sensible defaults
|
||
```console | ||
helm repo add cloudzero https://cloudzero.github.io/cloudzero-charts | ||
helm install cloudzero cloudzero/cloudzero-agent \ | ||
--set apiKey=<YOUR_CLOUDZERO_API_KEY>\ | ||
--set clusterName=<YOUR_CLUSTER_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.
it would be really nice to require less - we have a ticket for this here
Description
This is a swing at an ideal README size for agent installation. Ideally we provide a very simple installation that accounts for roughly 75% of the installations out there. This is a pipe dream of a configuration possibility, not an implementation of one. This is just meant to spark some lively discussions around what we could do to trim!