Skip to content
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

Merged
merged 4 commits into from Aug 6, 2019
Merged

Conversation

maimaisie
Copy link
Contributor

@maimaisie maimaisie commented Aug 6, 2019

This PR adds two new flags to the setup script:

  • -d for deploying (running kubectl apply); default to true and
  • -y for downloading yaml; default to true

Two reasons for these changes:

  1. When working with @drduke1 on the docs we realize for users who wants to customize the configuration, they currently run setup.sh which deploys everything with default settings, then they'll make changes to the downloaded yaml, and then redeploy. With this change, they can set -d false and it will only download the yaml file so they can make changes locally and run kubectl apply on their own.
  2. Before we add pre-installation hook to our Helm chart that sets up the collector and sources as part of 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:

  • default: deploy and download yaml
  • -d false: only download
  • -d false -y false: no deploy and no download (only set up the secrets for collector and sources)

rvmiller89
rvmiller89 previously approved these changes Aug 6, 2019
@@ -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>'
Copy link
Contributor

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.

Copy link
Contributor Author

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh good point.

@maimaisie maimaisie merged commit 040a10c into master Aug 6, 2019
@maimaisie maimaisie deleted the maisie-setup-sh branch August 6, 2019 22:56
* __-k &lt;cluster_name&gt;__ - 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 &lt;namespace&gt;__ - optional. Name of the Kubernetes namespace in which to deploy resources. If not specified, the namespace will default to `sumologic`.
* __-a &lt;boolean&gt;__ - 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 &lt;boolean&gt;__ - 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.
Copy link
Contributor

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 ..

Copy link
Contributor Author

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)

psaia pushed a commit to psaia/sumologic-kubernetes-collection that referenced this pull request May 25, 2021
* Comment out test pipeline

* comment out dev dependency
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants