Skip to content

Add OSDK Helm docs #13231

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Jan 11, 2019
Merged

Add OSDK Helm docs #13231

merged 1 commit into from
Jan 11, 2019

Conversation

adellape
Copy link
Contributor

@adellape adellape commented Jan 3, 2019

@openshift-ci-robot openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 3, 2019
@openshift-ci-robot openshift-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 4, 2019
@adellape adellape changed the title [WIP] Add OSDK Helm docs Add OSDK Helm docs Jan 4, 2019
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 4, 2019
* Create a Nginx Service if it does not exist.
* Create a Nginx Ingress if it is enabled and does not exist.
* Ensure that the Deployment, Service, and optional Ingress match the desired
configuration (e.g., replica count, image, service type, etc.) as specified by
Copy link
Contributor

Choose a reason for hiding this comment

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

s/etc./and so on

to provide customizations to a Helm chart's defaults, which are defined in the
Helm chart's `values.yaml` file.
+
Overriding these defaults is as simple as setting the desired values in the CR
Copy link
Contributor

Choose a reason for hiding this comment

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

Override these defaults by setting the desired values . . .

(It's good practice to avoid labelling processes as simple because if the user doesn't find the process simple, then they can feel silly or frustrated.)

example-nginx-b9phnoz9spckcrua7ihrbkrt1-f8f9c875d-ljbzl 1/1 Running 0 1m
----
+
Check that the Service port is set to 8080:
Copy link
Contributor

Choose a reason for hiding this comment

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

`8080`

example-nginx-b9phnoz9spckcrua7ihrbkrt1 3 3 3 3 1m
----
+
Check that the Service port is set to the default (80):
Copy link
Contributor

Choose a reason for hiding this comment

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

`80`

@ahardin-rh ahardin-rh added the peer-review-done Signifies that the peer review team has reviewed this PR label Jan 7, 2019
@ahardin-rh
Copy link
Contributor

@adellape Just a few minor nits from me. 👍

@adellape
Copy link
Contributor Author

adellape commented Jan 7, 2019

@joelanford PTAL for tech review.

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

Looks great overall. Just a few minor updates I'd suggest making.

your application instance and have its desired state match what is running. In
the case of a Helm-based Operator, the object's spec field is a list of
configuration options that are typically described in Helm's `values.yaml` file.
Instead of setting these values with flags using the Helm CLI (e.g., `elm
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Instead of setting these values with flags using the Helm CLI (e.g., `elm
Instead of setting these values with flags using the Helm CLI (e.g., `helm

$ operator-sdk up local
INFO[0000] Go Version: go1.10.3
INFO[0000] Go OS/Arch: linux/amd64
INFO[0000] operator-sdk Version: v0.1.1+git
Copy link
Member

Choose a reason for hiding this comment

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

I would suggest bumping the version number to v0.3.0+git. Same for the kubeconfig example below.

.Prerequisites

- Operator SDK CLI installed on the development workstation
- Operator Lifecycle Manager (OLM) installed on a Kubernetes-based cluster (v1.8
Copy link
Member

Choose a reason for hiding this comment

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

OLM is more of an addon than a prerequisite, right? I believe this is the only mention of OLM in this document.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, this is a leftover from reusing this list from elsewhere. Will remove.

or above to support the `apps/v1beta2` API group), for example {product-title}
4.0
- Access to the cluster using an account with `cluster-admin` permissions
- link:https://kubernetes.io/docs/tasks/tools/install-kubectl/[`kubectl`] v1.11.0+
Copy link
Member

Choose a reason for hiding this comment

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

Based on this comment and the bug it references, I think the minimum Kubernetes cluster version and kubectl version is 1.11.3.

@adellape
Copy link
Contributor Author

adellape commented Jan 8, 2019

Thx @ahardin-rh @joelanford, changes made.

@jianzhangbjz Can you take a look for QE review?

http://file.rdu.redhat.com/~adellape/010319/helm-sdk/operators/osdk-helm.html

Thank you!

Edit: Forgot we're gonna do QE later for 4.0 content. Merging as-is for now.

@adellape adellape added this to the Future Release milestone Jan 10, 2019
@adellape adellape merged commit b75c81e into openshift:enterprise-4.0 Jan 11, 2019
@jianzhangbjz
Copy link

@adellape I'm so sorry for the late to reply. I will review this document ASAP, thanks very much!

@adellape
Copy link
Contributor Author

/cherrypick master

@openshift-cherrypick-robot

@adellape: new pull request created: #13350

In response to this:

/cherrypick master

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@jianzhangbjz
Copy link

Looks great! Thanks!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
branch/enterprise-4.1 peer-review-done Signifies that the peer review team has reviewed this PR size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants