Skip to content

doc: quickstart for Helm projects and more details for the migration guide#3552

Merged
camilamacedo86 merged 4 commits intooperator-framework:masterfrom
camilamacedo86:helm-doc-quick
Jul 29, 2020
Merged

doc: quickstart for Helm projects and more details for the migration guide#3552
camilamacedo86 merged 4 commits intooperator-framework:masterfrom
camilamacedo86:helm-doc-quick

Conversation

@camilamacedo86
Copy link
Copy Markdown
Contributor

@camilamacedo86 camilamacedo86 commented Jul 26, 2020

Description of the change:

Motivation for the change:

#3327

@camilamacedo86 camilamacedo86 changed the title doc: add helm quickstart doc: quickstart for Helm projects Jul 26, 2020
@joelanford joelanford mentioned this pull request Jul 26, 2020
92 tasks
@camilamacedo86 camilamacedo86 added this to the v1.0.0 milestone Jul 27, 2020
@camilamacedo86 camilamacedo86 added kind/documentation Categorizes issue or PR as related to documentation. language/helm Issue is related to a Helm operator project labels Jul 27, 2020
@camilamacedo86 camilamacedo86 changed the title doc: quickstart for Helm projects doc: quickstart for Helm projects and more details for the migration guide Jul 28, 2020
Comment thread website/content/en/docs/building-operators/helm/quickstart.md Outdated
Copy link
Copy Markdown
Contributor

@bharathi-tenneti bharathi-tenneti left a comment

Choose a reason for hiding this comment

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

Some nits.

chart: helm-charts/nginx
# +kubebuilder:scaffold:watch
```

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bottom note about adding additional APIs, might be placed here, to be relevant.

Suggested change
**NOTE** Additional CR/CRD's can be added to the project by running, for example, the command :`operator-sdk create api --group=example --version=v1alpha1 --kind=AppService`

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I did not add it at the latest version. After check all again, I saw that it is at the end of the file already.

Comment thread website/content/en/docs/building-operators/helm/tutorial.md Outdated
Comment thread website/content/en/docs/building-operators/helm/quickstart.md Outdated
Comment thread test/test-framework/go.sum Outdated
Comment thread internal/scorecard/alpha/examples/custom-scorecard-tests/go.sum Outdated
Comment thread website/content/en/docs/building-operators/helm/migration.md Outdated
Comment thread website/content/en/docs/building-operators/helm/migration.md Outdated
Comment thread website/content/en/docs/building-operators/helm/quickstart.md Outdated
Comment thread website/content/en/docs/building-operators/helm/quickstart.md
Comment thread website/content/en/docs/building-operators/helm/quickstart.md Outdated
Comment thread website/content/en/docs/building-operators/helm/quickstart.md Outdated
Comment thread website/content/en/docs/building-operators/helm/quickstart.md Outdated
@joelanford
Copy link
Copy Markdown
Member

@camilamacedo86 Given how simple this quickstart will be, I'm thinking we should eliminate all of the headers and basically just list the commands one after the other, maybe with single sentence descriptions of what each command does.

I think we want this to be simple for people to copy and paste from, so combining several commands in a
console code block might be helpful. Like this

Initialize a project, create your first API, and build and push the operator image:
```console
mkdir demo-operator && cd demo-operator
operator-sdk init --plugins=helm
operator-sdk create api ...
make docker-build docker-push IMG=<repo>/<name>:<tag>
```

Install the CRDs and run the operator:
```console
make install
make deploy IMG=<repo>/<name>:<tag>
```

Create a sample custom resource and wait for the operator to create an nginx deployment:
```console
kubectl apply -f ./config/samples/...
kubectl get deployment -w | grep nginx-sample
```

Uninstall the operator
```console
make undeploy
```

@openshift-ci-robot openshift-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jul 28, 2020
@joelanford joelanford self-requested a review July 28, 2020 20:30
Comment thread website/content/en/docs/building-operators/helm/migration.md Outdated
Comment on lines +61 to +62
**Note** Ensure that you use the same values for the flags to recreate the same Helm chart and API. If you have
more than one chart or API you can add them via `operator-sdk create api` command. For further information check the [Helm Quickstart][quickstart].
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this change makes the doc less clear. I think we should revert this, and add an extra sentence about --version and --kind

Suggested change
**Note** Ensure that you use the same values for the flags to recreate the same Helm chart and API. If you have
more than one chart or API you can add them via `operator-sdk create api` command. For further information check the [Helm Quickstart][quickstart].
Now that we have our new project initialized, we need to re-create each of our APIs.
Using our API example from earlier (`cache.example.com`), we'll use `cache` for the
`--group` flag.
For `--version` and `--kind`, we use `spec.versions[0].name` and `spec.names.kind`, respectively.

more than one chart or API you can add them via `operator-sdk create api` command. For further information check the [Helm Quickstart][quickstart].

For each API in the existing project, run:
```sh
Copy link
Copy Markdown
Member

@joelanford joelanford Jul 29, 2020

Choose a reason for hiding this comment

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

This example create api codeblock needs to be updated. There is no longer an --api-version flag.

Comment on lines +125 to +126
In case your project has customizations in the `deploy/operator.yaml` then, it needs to be port to
`config/manager/manager.yaml`. However, note that the following env vars are no longer used.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
In case your project has customizations in the `deploy/operator.yaml` then, it needs to be port to
`config/manager/manager.yaml`. However, note that the following env vars are no longer used.
If your existing project has customizations in `deploy/operator.yaml`, they need to be ported to
`config/manager/manager.yaml`. If you are passing custom arguments in your deployment, make sure to also update `config/default/auth_proxy_patch.yaml`.
Note that the following environment variables are no longer used.


This guide walks through an example of building a simple nginx-operator powered by [Helm][helm-official] using tools and libraries provided by the Operator SDK.
This guide walks through an example of building a simple nginx operator
powered by Helm using tools and libraries provided by the Operator SDK.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should add the official Helm link back. I may have accidentally dropped that in the commit that I pushed.

Copy link
Copy Markdown
Member

@joelanford joelanford left a comment

Choose a reason for hiding this comment

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

/lgtm

After final comments are addressed.

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 29, 2020
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jul 29, 2020
@openshift-ci-robot
Copy link
Copy Markdown

New changes are detected. LGTM label has been removed.

@camilamacedo86 camilamacedo86 merged commit f2088cf into operator-framework:master Jul 29, 2020
@camilamacedo86 camilamacedo86 deleted the helm-doc-quick branch July 29, 2020 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/documentation Categorizes issue or PR as related to documentation. language/helm Issue is related to a Helm operator project

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants