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

Add CRD/APIService sections to CSV topic #15089

Merged
merged 1 commit into from
Sep 5, 2019

Conversation

adellape
Copy link
Contributor

@adellape adellape added this to the Future Release milestone May 30, 2019
@openshift-ci-robot openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 30, 2019
@adellape
Copy link
Contributor Author

@jianzhangbjz PTAL for QE review.


It is recommended to only list out the objects that are important to a human,
not an exhaustive list of everything you orchestrate. For example, ConfigMaps
that store internal state that should not be modified by a user should npt

Choose a reason for hiding this comment

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

Typing error: npt, shoud be not I guess.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

[id="osdk-apiservices_{context}"]
= Understanding your API services

As with CRDs, there are two types of APIServices that your Operator may use:

Choose a reason for hiding this comment

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

@ecordell Do we have the two sections for APIServices in the csv object? I'm sorry, but I haven't seen any csv which contains the owned/required for APIService in community-operators. I can only find the csv which contain owned/required for CRD.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cc @njhale since I believe you authored the upstream version via operator-framework/operator-lifecycle-manager#492.

Copy link

Choose a reason for hiding this comment

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

Yes, there are both sections for apiservices as well. I beleive the template service broker and ansible service broker make use of them?

But yes, the vast majority of operators are CRD-based (and this is a good thing)

Choose a reason for hiding this comment

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

ok, got it, thanks!

such as top-level database configuration in one object and a representation of
ReplicaSets in another. Each one should be listed out in the CSV file.

.Owned CRD fields

Choose a reason for hiding this comment

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

@ecordell Do we need to point out the must or optional fields? For example, etcd csv, there are no Group and ActionDescriptors fields.

Choose a reason for hiding this comment

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

@ecordell what do you think?

Choose a reason for hiding this comment

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

That makes sense to me; though the fields are defined required/optional in the schema for the api.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated Tables 3, 4, 5, and 6 with a Required/Optional column, based on what was listed as required for customresourcedefinitions and apiservicedefinitions in https://github.com/operator-framework/operator-lifecycle-manager/blob/master/manifests/0000_50_olm_03-clusterserviceversion.crd.yaml.

an etcd cluster (from an etcd Operator) to use for distributed locking and a
Postgres database (from a Postgres Operator) for data storage.

The Operator Lifecycle Manager (OLM) checks against the available CRDs and

Choose a reason for hiding this comment

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

@ecordell OLM will resolve and install the required CRD, right? Do we need to point out it here?

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

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

This is describing the "single-operator" workflow. If you have no catalogs and no subscriptions, OLM will still check that your required CRDs exist (if, for example, you have installed it outside of OLM)

@adellape adellape force-pushed the yourapiservices branch 2 times, most recently from fdd57b5 to db36950 Compare May 31, 2019 11:41
@adellape adellape changed the base branch from enterprise-4.1 to master June 4, 2019 17:03
@adellape
Copy link
Contributor Author

@vikram-redhat
Copy link
Contributor

@adellape added 4.2 as well, but remove if not applicable.

@jianzhangbjz
Copy link

@adellape Sorry for the late, I will try to review it ASAP.

@adellape adellape merged commit 0a3a9ea into openshift:master Sep 5, 2019
@adellape
Copy link
Contributor Author

adellape commented Sep 5, 2019

/cherrypick enterprise-4.1

@openshift-cherrypick-robot

@adellape: new pull request created: #16509

In response to this:

/cherrypick enterprise-4.1

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.

@adellape
Copy link
Contributor Author

adellape commented Sep 5, 2019

/cherrypick enterprise-4.2

@openshift-cherrypick-robot

@adellape: new pull request created: #16510

In response to this:

/cherrypick enterprise-4.2

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch/enterprise-4.1 branch/enterprise-4.2 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.

6 participants