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

Final phase of 'developing operators' docs #651

Merged
merged 6 commits into from
Aug 2, 2019
Merged

Conversation

alenkacz
Copy link
Contributor

This is the final increment of developing operators docs. So if you still feel like there's something essential missing, this is your place to voice that.

To give all credits where they are due, some parts are my own, some I borrowed from @realmbgl and some (but very little) are from the DC/OS SDK docs.

Fixes #391

@alenkacz alenkacz requested a review from gerred as a code owner July 25, 2019 13:42
Copy link

@joaompinto joaompinto left a comment

Choose a reason for hiding this comment

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

Looks good

Copy link
Member

@kensipe kensipe left a comment

Choose a reason for hiding this comment

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

I'm not sure I like this doc living in developing-operators.md
Based on title of PR I was expecting this to be specific to plans and phases... there is more info here on other subjects as there is on plan / phases. we should be more verbose on plans and phases.
Like: if there is another arbitrary plan... how do I invoke that? or the fact that recommend that an upgrade of a stateful operator is to "backup", "upgrade", "restore".


Plans orchestrate tasks through `phases` and `steps`.

Each Plan is a tree with a fixed three-level hierarchy of the plan itself, its phases, and then steps within those phases. The choice of three levels was arbitrarily chosen as “enough levels for anybody”. This three-level hierarchy can look as follows:
Copy link
Member

Choose a reason for hiding this comment

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

  1. I don't think the levels are arbitrary and
  2. "enough levels for anybody" seems not supportive and doesn't seems like it needs to be in our official documentation.

The hierarch is:

  1. a "plan" which is a named collection of phases and steps
  2. "phases" is a collection of steps AND also provides an ability to group serial steps and concurrent steps with a point of demarcation that allows for health before moving forward. This point of validation at the end of phase could be a human verification in an upgrade flow.
  3. "Steps" is the actual action to be applied via tasks as you describe.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I copied that sentence from SDK docs, I am fine with droping it. It does not offend me, it shows that we did some design behind the three levels and it's the case right now, we have three levels, no less no more. But I am fine droping that sentence, I don't have any personal attachment to that since it's taken from SDK docs :)

Choose a reason for hiding this comment

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

I think we could probably drop both sentences entirely. As a reader, I think it's a brand new concept that they probably don't have an opinion on - and if they do, hopefully it's in a KEP somewhere.

```

Plans allow operators to see what the operator is currently doing, and to visualize the broader operation such as for a config rollout. The fixed structure of that information meanwhile makes it straightforward to build UIs and tooling on top.

Copy link
Member

Choose a reason for hiding this comment

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

this is a good place to show kudo plan status output

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure that should be in developing operators :/ maybe we need another portion of the docs for running that operator and then seeing how the plan is executing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

looking at the issues, that is definitely something I would expect in here #395 ... so basically here you create your operator and in that other docs you actually tell people how to run it and monitor it. We could link that other doc from the end of this doc :) that would be my proposal

@alenkacz
Copy link
Contributor Author

alenkacz commented Jul 26, 2019

I think there is some level of knowledge you need to know about plans etc. to be able to write your own operator without jumping between several pages of docs. I think some repetition is good. I am not agains having a separate plans/phases docs outside of this.

Seems like you have clear vision of how the plans docs should look like, maybe you should do a draft then?

Considering this is guide that should help you develop your own operator, do you still consider this as a rejection? It does not aim to be exhaustive docs on plans/phases concept, although I feel like it covers it pretty nicely.

@alenkacz alenkacz changed the title Plans/phases docs, finish developing operator docs Final phase of 'developing operators' docs Jul 26, 2019
@alenkacz
Copy link
Contributor Author

@kensipe also you're right, this is not plans/phases docs, it's just that portion of the developing operators docs.I changed the headline of this PR...

description: my first super awesome operator
version: "5.7"
kudoVersion: ">= 0.2.0"
kubeVersion: ">= 1.14"

Choose a reason for hiding this comment

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

kubernetesVersion?

url: https://github.com/myoperator/myoperator
```

Most of these are provided as a form of documentation. `kudoVersion` and `kubeVersion` use semver constraints to define minimal or maximal version of kubernetes or kudo that this operator supports. Under the hood, we use [this library](https://github.com/Masterminds/semver) to evaluate the constraints.

Choose a reason for hiding this comment

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

IMO, I don't think we need to reference the library as it is an implementation detail - if the library has nuances that Kubernetes / semantic version doesn't have then that sounds like a bug.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

well the reasoning here was that I think semver constraints have no specifications, people implement it differently. Ideally I should probably explain what is the format that kudo expects, I made it easier for myself by linking the library :D I can change that... no strong opinion, just laziness :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah by looking at their docs I would have to port over a lot of readme about how to define ranges etc. :/

docs/developing-operators.md Outdated Show resolved Hide resolved

Plans orchestrate tasks through `phases` and `steps`.

Each Plan is a tree with a fixed three-level hierarchy of the plan itself, its phases, and then steps within those phases. The choice of three levels was arbitrarily chosen as “enough levels for anybody”. This three-level hierarchy can look as follows:

Choose a reason for hiding this comment

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

I think we could probably drop both sentences entirely. As a reader, I think it's a brand new concept that they probably don't have an opinion on - and if they do, hopefully it's in a KEP somewhere.

docs/developing-operators.md Outdated Show resolved Hide resolved
alenkacz and others added 2 commits July 31, 2019 15:30
Co-Authored-By: Justin Taylor-Barrick <46463088+jbarrick-mesosphere@users.noreply.github.com>
Co-Authored-By: Justin Taylor-Barrick <46463088+jbarrick-mesosphere@users.noreply.github.com>
@kudo-ci kudo-ci added the size/M label Jul 31, 2019
@@ -84,6 +84,75 @@ Now your first operator is ready and you can install it to cluster. You can do i

For simplicity if you want to install what we created here without actually replicating it on your filesystem your can just clone KUDO repository and then run `kubectl kudo install ./config/samples/first-operator`.

## Operator.yaml file

This is the main piece of every operator. It consists of two main parts. First one defines metadata about your operator.
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
This is the main piece of every operator. It consists of two main parts. First one defines metadata about your operator.
This is the main required file of every operator. It consists of two main parts. First one defines metadata about your operator.

@kudo-ci
Copy link

kudo-ci commented Aug 2, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alenkacz, kensipe

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kensipe
Copy link
Member

kensipe commented Aug 2, 2019

/lgtm

@kudo-ci kudo-ci added the lgtm label Aug 2, 2019
@kudo-ci kudo-ci merged commit 3c3346a into master Aug 2, 2019
@kudo-ci kudo-ci deleted the av/plan-phases-docs branch August 2, 2019 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create Framework Developer Guide
5 participants