-
Notifications
You must be signed in to change notification settings - Fork 183
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
Optionally deploy and download yaml in setup.sh #124
Conversation
@@ -4,7 +4,7 @@ set -e | |||
usage() { | |||
echo | |||
echo 'Usage:' | |||
echo ' setup.sh [-c collector-name] [-k cluster-name] [-n namespace] [-a useAlpha] <endpoint> <access-id> <access-key>' | |||
echo ' setup.sh [-c collector-name] [-k cluster-name] [-n namespace] [-a use-alpha] [-d deploy] [-y download-yaml] <endpoint> <access-id> <access-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.
technically changing parameter name is a breaking change in case anyone has integrated this script into their automation, but since it's so new it's probably safe to change to use-alpha
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.
it is actually not the parameter name, just a placeholder for the parameter value. the name is just -a
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.
oh good point.
* __-k <cluster_name>__ - optional. Name of the Kubernetes cluster that will be attached to logs and events as metadata. If not specified, it will be named as `kubernetes-<timestamp>`. For metrics, specify the cluster name in the `prometheus-overrides.yaml` provided for the prometheus operator; further details in [step 2](#step-2-configure-prometheus). | ||
* __-n <namespace>__ - optional. Name of the Kubernetes namespace in which to deploy resources. If not specified, the namespace will default to `sumologic`. | ||
* __-a <boolean>__ - optional. Set this to true if you want to deploy with the latest alpha version. If not specified, the latest release will be deployed. | ||
* __-d <boolean>__ - optional. Set this to false to only set up the Sumo Collector and Sources and download the YAML file, but not to deploy so you can customize the YAML file. If not specified, the default configuration will deploy. |
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.
Can we make -a
and -d
options instead of parameters? It doesn't make sense that user have to pass true/false there. Also the default values are not consistent with each other ..
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.
From the email this morning the customer mentioned
option parsing needs to either be all flags or all positional – I added a couple extra parameters and the option to pass in a local file since we had to override the
image:
fields (see code snippet below)
Does this mean he doesn't want a mix of parameters and flags? When I added -d
and -y
I wanted to be consistent with the existing ones. I'm not against changing them (and the default values)
* Comment out test pipeline * comment out dev dependency
This PR adds two new flags to the setup script:
-d
for deploying (runningkubectl apply
); default to true and-y
for downloading yaml; default to trueTwo reasons for these changes:
-d false
and it will only download the yaml file so they can make changes locally and runkubectl apply
on their own.helm install
, Helm users will need to run the setup script with-d false -y false
before they install the chart.To summarize, these are the conditions and the behaviors:
-d false
: only download-d false -y false
: no deploy and no download (only set up the secrets for collector and sources)