Skip to content

Conversation

@rajatchopra
Copy link

New document/guide in the form of a detailed FAQ for operator authors wanting to integrate their operators with the cluster payload generated by CVO.
Excerpted from internal documentation.

@openshift-ci-robot openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 8, 2018
@rajatchopra
Copy link
Author

@wking Feedback please.
Making public of the internal doc, for benefit of PR openshift/installer#377

Copy link
Member

@wking wking 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 to me. A few minor nits inline.


In your deployment you can reference the latest development version of your operator image (quay.io/openshift/origin-machine-api-operator:latest). If you have other hard-coded image strings, try to put them as environment variables on your deployment or as a config map.

Your manifests will be applied in alphabetical order by the CVO, so name your files in the order you want them run.
Copy link
Member

Choose a reason for hiding this comment

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

Shift this line into the next section?


Your manifests will be applied in alphabetical order by the CVO, so name your files in the order you want them run.

### Names of manifest files:
Copy link
Member

Choose a reason for hiding this comment

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

You can drop the trailing colon.

99_ingress-operator_01_roles.yaml
99_ingress-operator_02_deployment.yaml

## How do I get added as a special run level?
Copy link
Member

Choose a reason for hiding this comment

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

This belongs as a sibling of the previous subsection, since it discusses /manifests content. It should be a ### header.

## How do I ensure the right images get used by my manifests?
Your manifests can contain a tag to the latest development image published by Origin. You’ll annotate your manifests by creating a file that identifies those images.

Assume you have two images in your manifests - “quay.io/openshift/origin-ingress-operator:latest” and “quay.io/openshift/origin-haproxy-router:latest”. Those correspond to the following tags “ingress-operator” and “haproxy-router” when the CI runs.
Copy link
Member

Choose a reason for hiding this comment

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

These image names and tags should probably be backticked instead of being curly-quoted.

@rajatchopra rajatchopra force-pushed the docs branch 2 times, most recently from bc1185c to 9cbd960 Compare October 8, 2018 23:17
@rajatchopra
Copy link
Author

@wking All feedback incorporated. Thanks.

@wking
Copy link
Member

wking commented Oct 8, 2018

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 8, 2018
@smarterclayton
Copy link
Contributor

/test e2e-aws

Copy link
Contributor

@crawford crawford left a comment

Choose a reason for hiding this comment

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

One small NIT.


0000_<runlevel>_<dash-separated_component>-<manifest_filename>

For example, the Kube core operators run in runlevel 10 and have filenames like
Copy link
Contributor

Choose a reason for hiding this comment

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

For example, the Kube operators run in runlevels 10-19 and have filenames like

Copy link
Author

Choose a reason for hiding this comment

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

Fixed.

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Oct 9, 2018
@abhinavdahiya
Copy link
Contributor

abhinavdahiya commented Oct 9, 2018

dbe7eeb has title docs/dev: can you fix that;

usually @wking would format the commits like

code-area: short description

detailed description <optional>

which is super helpful.

New document/guide in the form of a detailed FAQ for operator authors wanting to integrate their operators with the cluster payload generated by CVO.
@abhinavdahiya
Copy link
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 9, 2018
@crawford crawford changed the title Doc on how to integrate a second level operator docs/dev: Second level operator FAQ Oct 9, 2018
@crawford
Copy link
Contributor

crawford commented Oct 9, 2018

/lgtm

@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinavdahiya, crawford, rajatchopra, wking

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

The pull request process is described here

Details 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

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 9, 2018
@openshift-merge-robot openshift-merge-robot merged commit 46695ca into openshift:master Oct 9, 2018
wking added a commit to wking/cluster-version-operator that referenced this pull request Nov 27, 2018
It feels like the wording wanted this to be an enumerated list, but
the text landed without that markup in e046cd7 (docs/dev: Second
level operator FAQ, 2018-10-08, openshift#34).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants