New doc on how to integrate an operator #377
Conversation
openshift-ci-robot
added
size/M
| Upshot of the above document is that the operator should keep its static manifest files in manifests/ folder of the operator repository and manage image references through an image-references file. The order of creation of the files is alphabetical, so the files should be numbered like below to effectively create the service account before the deployment. | ||
| : | ||
| ``` | ||
| 0000_70_my-operator-name_04_service-account.yaml |
There was a problem hiding this comment.
This is not recommended. Only special operators require this.
cc @smarterclayton
The files should be <internal order>_<name>.yaml like here https://github.com/openshift/machine-config-operator/tree/master/install
There was a problem hiding this comment.
00-namaspace.yaml
...
03-roles.yaml
04-serviceaccount.yaml
05-deplopment.yaml
| Take the static files and ready it for the Cluster Version Operator (CVO) to pick it up as its payload. See this document: | ||
| https://docs.google.com/document/d/1CGZVEyuloZ9oD4NUArW6dnEpi0WFc6BP2tqPSwFZTCY/edit#heading=h.1zgrwxmpgxbr | ||
|
|
||
| Upshot of the above document is that the operator should keep its static manifest files in manifests/ folder of the operator repository and manage image references through an image-references file. The order of creation of the files is alphabetical, so the files should be numbered like below to effectively create the service account before the deployment. |
There was a problem hiding this comment.
Upshot of the above document is that the operator should keep its static manifest files in manifests/ folder of the operator repository and manage image references through an image-references file.
The document above requires more than keeping in /manifests of the repo. I would recommend not duplicate here.
The order of creation of the files is alphabetical, so the files should be numbered like below to effectively create the service account before the deployment.
ClusterVersionOperator applies the manifests to cluster in alphabetical order. So it is recommended that manifests be order of dependencies.
For example,
- a deployment requires service account, roles etc and,
- everything requires the namespace to exist.
So the order of the manifests should be enforces like:
There was a problem hiding this comment.
The document above requires more than keeping in
/manifestsof the repo. I would recommend not duplicate here.
Agreed on not duplicating here. And any references to this that we keep should talk about the operator images; the operator Git repository doesn't come into it.
But if we punt this portion of the docs to external docs (whether in a Google Doc or github.com/openshift/cluster-version-operator Markdown), what's left to talk about here? Just:
Almost everyone should use external docs. {details about when that won't work}. {where to put things when you add them to this repo}.
| Upshot of the above document is that the operator should keep its static manifest files in manifests/ folder of the operator repository and manage image references through an image-references file. The order of creation of the files is alphabetical, so the files should be numbered like below to effectively create the service account before the deployment. | ||
| : | ||
| ``` | ||
| 0000_70_my-operator-name_04_service-account.yaml |
There was a problem hiding this comment.
00-namaspace.yaml
...
03-roles.yaml
04-serviceaccount.yaml
05-deplopment.yaml
| 0000_70_my-operator-name_04_service-account.yaml | ||
| 0000_70_my-operator-name_05_deployment.yaml | ||
| ``` | ||
| where, 0000 is the global run-level, 70 is the operator level, and 04/05 is the internal ordering of the resource manifests. |
There was a problem hiding this comment.
drop this. not required for most operators other than internal ordering.
| Example: | ||
| https://github.com/openshift/machine-config-operator/blob/e932afdec07dc86d5b643590164f86811e205c57/pkg/operator/operator.go#L272 | ||
|
|
||
| After discovering the InstallConfig, the operator pod can create its own configuration in memory. e.g. the discoverMCOConfig returns the configuration for the operator as it is discovered rather than picking it up from an actual configmap. |
There was a problem hiding this comment.
- either keep it in memory as users shouldn't be editing it.
- push the discovered config to api, as users might change this in future.
There was a problem hiding this comment.
@abhinavdahiya do you have an example of 'push the discovered config to api, as users might change this in future'?
|
@rajatchopra: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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. I understand the commands that are listed here. |
There was a problem hiding this comment.
A few nits inline, take 'em if they sound useful ;).
More broadly, I'm not clear on how this is scoped vs. the Google Doc, and I feel like the CVO repository is feeling lonely and left-out. @smarterclayton, can you provide more details about where you see these docs being maintained long-term? If it's in multiple locations, I think we need clear scoping about what goes where so we can give good cross-links to guide newcomers between locations.
| @@ -0,0 +1,53 @@ | |||
| ## How to add a new operator to the installer? | |||
There was a problem hiding this comment.
nit: The page title should be h1 (one #), and subsections should increment by one level. It's not our fault if we think GitHub styles them too big ;).
Also, do we expect other levels of operator docs besides this? It seems unlikely to me, in which case I'd prefer docs/dev/operators.md and
# Operator integrationThere was a problem hiding this comment.
h1 is indeed to big for a smallish document like this one. Too bad if someone is forced into a telescopic lens. Can I stick with h2/h4?
There was a problem hiding this comment.
No. Headings should not be used for styling. That's what CSS is for. Just stick to the standard hierarchy.
There was a problem hiding this comment.
Ack.
Convinced? No.
Complied? Yes.
| Take the static files and ready it for the Cluster Version Operator (CVO) to pick it up as its payload. See this document: | ||
| https://docs.google.com/document/d/1CGZVEyuloZ9oD4NUArW6dnEpi0WFc6BP2tqPSwFZTCY/edit#heading=h.1zgrwxmpgxbr | ||
|
|
||
| Upshot of the above document is that the operator should keep its static manifest files in manifests/ folder of the operator repository and manage image references through an image-references file. The order of creation of the files is alphabetical, so the files should be numbered like below to effectively create the service account before the deployment. |
There was a problem hiding this comment.
The document above requires more than keeping in
/manifestsof the repo. I would recommend not duplicate here.
Agreed on not duplicating here. And any references to this that we keep should talk about the operator images; the operator Git repository doesn't come into it.
But if we punt this portion of the docs to external docs (whether in a Google Doc or github.com/openshift/cluster-version-operator Markdown), what's left to talk about here? Just:
Almost everyone should use external docs. {details about when that won't work}. {where to put things when you add them to this repo}.
|
|
||
| *What to do for my dynamic template files?* | ||
|
|
||
| An operator should auto-discover the install config rather than expand the templates through the installer-src integration (see alternative). Simply use the apiserver access to get the install-config as a config map. The config map is stored by the name ‘cluster-config-v1’ in the ‘kube-system’ namespaces. |
There was a problem hiding this comment.
I think it's worth spending a bit more time (wherever these "standard approach" docs live), talking about how this wires in. As I understand it, you'd take the /manifests route with enough static content to get your operator launched, and then you pull everything else you need (e.g. the install config) live from well-known locations. You focus on the last part of that here, and the Google Doc focuses on the first part. I don't see anywhere that's tying them together, although the MCO link you have below probably gives a good example for folks who want to read the source ;).
There was a problem hiding this comment.
I have done a bit of a shift up to tie the two methods (one for static, the other for templates). Review again please.
There was a problem hiding this comment.
I have done a bit of a shift up to tie the two methods (one for static, the other for templates).
This paragraph looks largely unchanged from 4f66f97 -> 2650b11. I was thinking of an outline more like:
The recommended way
...Everyone following this integration path will need static manifests/configs (as discussed in the Google Doc) to launch their operator...
Dynamic configuration
Some operators will need dynamic configuration to accomplish their task. These operators should set up their static manifests/configs to launch their operator, and then have their operator fill in its configuration by pulling information from cluster resources. For example, you can pull the install-config as a config map:
...
| Create a new operator asset, and fill up the Dependencies, Name and Generate functions. The Dependencies might contain InstallConfig as an example. In the Generate function, create the config files as asset contents. For the config/manifest’s actual structure, one can choose to vendor the operator’s github pkg and return the entire list of configs and manifests in the Generate function. | ||
|
|
||
| - Template files | ||
| In the pkg/asset/manifests/content/tectonic directory, place the templatized files as golang variables. Then modify pkg/asset/manifests/tectonic.go to expand the template. Expand templateData in template.go for filling up the template variables. |
There was a problem hiding this comment.
nit: "templateized files" -> "templates"?
And including the paths and variable names here seems brittle. But it's probably fine until we have commits we can link to showing good examples of adding a new operator so folks can pattern-match.
There was a problem hiding this comment.
Fixed.
Agree on the paths and variable names. Will be intractable at some point.
rajatchopra
force-pushed
the
patch-1
branch
5 times, most recently
from
2c6f306 to
2650b11
Compare
|
/approve |
wking
dismissed
abhinavdahiya’s stale review
He's approved this PR since: #377 (comment)
|
Can this be |
Renamed the file. |
@smarterclayton, would that be ok with you? If not, do we have a plan for public integration docs for "The recommended way"? |
Quick link to the doc in question here: https://docs.google.com/document/d/1CGZVEyuloZ9oD4NUArW6dnEpi0WFc6BP2tqPSwFZTCY/ |
|
We should probably scrub the doc, get any final short term feedback in, and
then make public. Alternatively we can just grab the “how do I” segments
out and put them into docs directly.
On Oct 4, 2018, at 4:50 PM, Rajat Chopra <[email protected]> wrote:
The doc is not public, but I guess the answer is have a PR in CVO repo with
the contents of those docs
@smarterclayton <https://github.com/smarterclayton>, would that be ok with
you? If not, do we have a plan for public integration docs for "The
recommended way"?
Quick link to the doc in question here:
https://docs.google.com/document/d/1CGZVEyuloZ9oD4NUArW6dnEpi0WFc6BP2tqPSwFZTCY/
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#377 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABG_pzpIZHCo7rohyrO16NIrLBYquZFZks5uhnSNgaJpZM4XAMmo>
.
|
docs/dev/operators.md
Outdated
| ## The recommended way | ||
| The static and template files of your operator need to be dealt with separately. | ||
|
|
||
| *Static files* |
There was a problem hiding this comment.
Maybe use a ### headline here?
docs/dev/operators.md
Outdated
| ``` | ||
| where, 04/05 is the internal ordering of the resource manifests. | ||
|
|
||
| *What to do for the dynamic template files?* |
There was a problem hiding this comment.
Maybe use a ### headline here?
docs/dev/operators.md
Outdated
| *What to do for the dynamic template files?* | ||
|
|
||
| An operator should auto-discover the install config rather than expand the templates through the installer integration (see alternative). Simply use the apiserver access to get the install-config as a config map. The config map is stored by the name ‘cluster-config-v1’ in the ‘kube-system’ namespaces. | ||
| Psuedo code: |
docs/dev/operators.md
Outdated
| After discovering the InstallConfig, the operator pod can do two things: | ||
|
|
||
| - create its own configuration in memory as users should not be editing it | ||
| - or, push the discovered config to an API as the operators users might want to change it in the future |
docs/dev/operators.md
Outdated
| - create its own configuration in memory as users should not be editing it | ||
| - or, push the discovered config to an API as the operators users might want to change it in the future | ||
|
|
||
| See example of the configuration for the operator being discovered rather than from a configmap: |
docs/dev/operators.md
Outdated
| Such operators need to be directly integrated in the installer's [`manifests` package](../../pkg/asset/manifests). Within this, there are two ways to get the manifests/config files integrated: | ||
|
|
||
| - A new asset for the operator | ||
| Create a new operator asset, and fill up the Dependencies, Name, Load and Generate functions. The Dependencies might contain InstallConfig as an example. In the Generate function, create the config files as asset contents. For the config/manifest’s actual structure, one can choose to vendor the operator’s github pkg and return the entire list of configs and manifests in the Generate function. |
There was a problem hiding this comment.
What is the actual choice here in the last sentence, i.e. what's the alternative to "vendor the operator’s github pkg and return the entire list of configs and manifests in the Generate function"?
docs/dev/operators.md
Outdated
|
|
||
| Also remember that the order of creation of the files is alphabetical, so the files should be numbered like below to effectively create the service account before the deployment. | ||
| ``` | ||
| 00-namaspace.yaml |
docs/dev/operators.md
Outdated
| For static files use the Cluster Version Operator (CVO) payload mechanism. There is a particular way to keep the manifest files so that the CVO update payload can pick it up. | ||
| See this document: | ||
|
|
||
| https://docs.google.com/document/d/1CGZVEyuloZ9oD4NUArW6dnEpi0WFc6BP2tqPSwFZTCY/edit#heading=h.1zgrwxmpgxbr |
There was a problem hiding this comment.
You can replace this reference, and possibly the whole recommended-way section, with a link to here now that openshift/cluster-version-operator#34 has landed.
rajatchopra
force-pushed
the
patch-1
branch
2 times, most recently
from
c734231 to
4f8e7ac
Compare
This document is meant for an operator author, who wants to integrate a second-level operator into the installer. The document is a guide on all the possible and acceptable methods.
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: abhinavdahiya, rajatchopra 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 |
This content is descended from openshift/installer@9c8a77dd (docs/dev: Doc on how to integrate an operator, 2018-09-28, openshift/installer#377), but handling dynamic configuration is a generic issue, so the docs should live in the generic CVO repository. This will make these docs more discoverable. It will also allow the installer docs to focus on installer-specific issues for the small subset of operators that need direct installer bindings.
A document to help an operator developer to integrate the operator with the installer. It covers what files to keep where so that configurations can be properly generated.