OSDOCS-391: Making a few OSDK updates #14592

bergerhoffer · 2019-04-25T19:22:38Z

@openshift/team-documentation For peer review please.

Preview: http://file.rdu.redhat.com/~ahoffer/2019/OSDOCS-391/applications/operator_sdk/osdk-getting-started.html#managing-memcached-operator-using-olm-osdk-getting-started

bergerhoffer · 2019-04-25T20:32:28Z

@hasbro17 Could you please review these updates to the product docs based on your PR: operator-framework/getting-started#36

modules/managing-memcached-operator-using-olm.adoc

hasbro17

LGTM.

Just wanted to point out that this will be followed by an overhaul of the OLM section since even with these fixes the guide is still a little outdated in how we use OLM.
operator-framework/getting-started#37

bergerhoffer · 2019-04-26T02:25:43Z

Thanks for the review, and the heads up @hasbro17. Will operator-framework/getting-started#37 be done as a part of https://jira.coreos.com/browse/OSDK-358, or by a separate JIRA?

hasbro17 · 2019-04-26T18:26:46Z

That will be a separate ticket.

kalexand-rh

Couple of small things and a question. It looks good!

modules/managing-memcached-operator-using-olm.adoc

jianzhangbjz · 2019-05-13T03:32:25Z

modules/managing-memcached-operator-using-olm.adoc

+    - default
+----
+
+.. Apply the Operator's CSV manifest to the specified namespace in the cluster:
 +
 ----
 $ curl -Lo memcachedoperator.0.0.1.csv.yaml https://raw.githubusercontent.com/operator-framework/getting-started/master/memcachedoperator.0.0.1.csv.yaml


Here, we used the predefined manifest file for this guide. I tried it with using this new generated csv file: deploy/olm-catalog/memcached-operator/0.0.1/memcached-operator.v0.0.1.clusterserviceversion.yaml. Note: replace the namespace placeholder to default.
But get errors below:

mac:memcached-operator jianzhang$ oc apply -f deploy/olm-catalog/memcached-operator/0.0.1/memcached-operator.v0.0.1.clusterserviceversion.yaml The ClusterServiceVersion "memcached-operator.v0.0.1" is invalid: []: Invalid value: map[string]interface {}{"apiVersion":"operators.coreos.com/v1alpha1", "kind":"ClusterServiceVersion", "metadata":map[string]interface {}{"generation":1, "uid":"b7e2c272-752f-11e9-b23d-029006402c82", "name":"memcached-operator.v0.0.1", "namespace":"default", "creationTimestamp":"2019-05-13T03:32:22Z", "annotations":map[string]interface {}{"alm-examples":"[{\"apiVersion\":\"cache.example.com/v1alpha1\",\"kind\":\"Memcached\",\"metadata\":{\"name\":\"example-memcached\"},\"spec\":{\"size\":3}}]", "capabilities":"Basic Install", "kubectl.kubernetes.io/last-applied-configuration":"{\"apiVersion\":\"operators.coreos.com/v1alpha1\",\"kind\":\"ClusterServiceVersion\",\"metadata\":{\"annotations\":{\"alm-examples\":\"[{\\\"apiVersion\\\":\\\"cache.example.com/v1alpha1\\\",\\\"kind\\\":\\\"Memcached\\\",\\\"metadata\\\":{\\\"name\\\":\\\"example-memcached\\\"},\\\"spec\\\":{\\\"size\\\":3}}]\",\"capabilities\":\"Basic Install\"},\"name\":\"memcached-operator.v0.0.1\",\"namespace\":\"default\"},\"spec\":{\"apiservicedefinitions\":{},\"customresourcedefinitions\":{\"owned\":[{\"kind\":\"Memcached\",\"name\":\"memcacheds.cache.example.com\",\"version\":\"v1alpha1\"}]},\"description\":\"Placeholder description\",\"displayName\":\"Memcached Operator\",\"install\":{\"spec\":{\"deployments\":[{\"name\":\"memcached-operator\",\"spec\":{\"replicas\":1,\"selector\":{\"matchLabels\":{\"name\":\"memcached-operator\"}},\"strategy\":{},\"template\":{\"metadata\":{\"labels\":{\"name\":\"memcached-operator\"}},\"spec\":{\"containers\":[{\"command\":[\"memcached-operator\"],\"env\":[{\"name\":\"WATCH_NAMESPACE\",\"valueFrom\":{\"fieldRef\":{\"fieldPath\":\"metadata.namespace\"}}},{\"name\":\"POD_NAME\",\"valueFrom\":{\"fieldRef\":{\"fieldPath\":\"metadata.name\"}}},{\"name\":\"OPERATOR_NAME\",\"value\":\"memcached-operator\"}],\"image\":\"REPLACE_IMAGE\",\"imagePullPolicy\":\"Always\",\"name\":\"memcached-operator\",\"resources\":{}}],\"serviceAccountName\":\"memcached-operator\"}}}}],\"permissions\":[{\"rules\":[{\"apiGroups\":[\"\"],\"resources\":[\"pods\",\"services\",\"endpoints\",\"persistentvolumeclaims\",\"events\",\"configmaps\",\"secrets\"],\"verbs\":[\"*\"]},{\"apiGroups\":[\"\"],\"resources\":[\"namespaces\"],\"verbs\":[\"get\"]},{\"apiGroups\":[\"apps\"],\"resources\":[\"deployments\",\"daemonsets\",\"replicasets\",\"statefulsets\"],\"verbs\":[\"*\"]},{\"apiGroups\":[\"monitoring.coreos.com\"],\"resources\":[\"servicemonitors\"],\"verbs\":[\"get\",\"create\"]},{\"apiGroups\":[\"apps\"],\"resourceNames\":[\"memcached-operator\"],\"resources\":[\"deployments/finalizers\"],\"verbs\":[\"update\"]},{\"apiGroups\":[\"cache.example.com\"],\"resources\":[\"*\"],\"verbs\":[\"*\"]}],\"serviceAccountName\":\"memcached-operator\"}]},\"strategy\":\"deployment\"},\"installModes\":[{\"supported\":true,\"type\":\"OwnNamespace\"},{\"supported\":true,\"type\":\"SingleNamespace\"},{\"supported\":false,\"type\":\"MultiNamespace\"},{\"supported\":true,\"type\":\"AllNamespaces\"}],\"maturity\":\"alpha\",\"provider\":{},\"version\":\"0.0.1\"}}\n"}}, "spec":map[string]interface {}{"apiservicedefinitions":map[string]interface {}{}, "customresourcedefinitions":map[string]interface {}{"owned":[]interface {}{map[string]interface {}{"version":"v1alpha1", "kind":"Memcached", "name":"memcacheds.cache.example.com"}}}, "description":"Placeholder description", "displayName":"Memcached Operator", "installModes":[]interface {}{map[string]interface {}{"supported":true, "type":"OwnNamespace"}, map[string]interface {}{"supported":true, "type":"SingleNamespace"}, map[string]interface {}{"supported":false, "type":"MultiNamespace"}, map[string]interface {}{"supported":true, "type":"AllNamespaces"}}, "version":"0.0.1", "install":map[string]interface {}{"strategy":"deployment", "spec":map[string]interface {}{"deployments":[]interface {}{map[string]interface {}{"name":"memcached-operator", "spec":map[string]interface {}{"strategy":map[string]interface {}{}, "template":map[string]interface {}{"metadata":map[string]interface {}{"labels":map[string]interface {}{"name":"memcached-operator"}}, "spec":map[string]interface {}{"containers":[]interface {}{map[string]interface {}{"imagePullPolicy":"Always", "name":"memcached-operator", "resources":map[string]interface {}{}, "command":[]interface {}{"memcached-operator"}, "env":[]interface {}{map[string]interface {}{"valueFrom":map[string]interface {}{"fieldRef":map[string]interface {}{"fieldPath":"metadata.namespace"}}, "name":"WATCH_NAMESPACE"}, map[string]interface {}{"name":"POD_NAME", "valueFrom":map[string]interface {}{"fieldRef":map[string]interface {}{"fieldPath":"metadata.name"}}}, map[string]interface {}{"name":"OPERATOR_NAME", "value":"memcached-operator"}}, "image":"REPLACE_IMAGE"}}, "serviceAccountName":"memcached-operator"}}, "replicas":1, "selector":map[string]interface {}{"matchLabels":map[string]interface {}{"name":"memcached-operator"}}}}}, "permissions":[]interface {}{map[string]interface {}{"rules":[]interface {}{map[string]interface {}{"apiGroups":[]interface {}{""}, "resources":[]interface {}{"pods", "services", "endpoints", "persistentvolumeclaims", "events", "configmaps", "secrets"}, "verbs":[]interface {}{"*"}}, map[string]interface {}{"apiGroups":[]interface {}{""}, "resources":[]interface {}{"namespaces"}, "verbs":[]interface {}{"get"}}, map[string]interface {}{"resources":[]interface {}{"deployments", "daemonsets", "replicasets", "statefulsets"}, "verbs":[]interface {}{"*"}, "apiGroups":[]interface {}{"apps"}}, map[string]interface {}{"apiGroups":[]interface {}{"monitoring.coreos.com"}, "resources":[]interface {}{"servicemonitors"}, "verbs":[]interface {}{"get", "create"}}, map[string]interface {}{"apiGroups":[]interface {}{"apps"}, "resourceNames":[]interface {}{"memcached-operator"}, "resources":[]interface {}{"deployments/finalizers"}, "verbs":[]interface {}{"update"}}, map[string]interface {}{"apiGroups":[]interface {}{"cache.example.com"}, "resources":[]interface {}{"*"}, "verbs":[]interface {}{"*"}}}, "serviceAccountName":"memcached-operator"}}}}, "maturity":"alpha", "provider":map[string]interface {}{}}}: validation failure list: spec.customresourcedefinitions.owned.displayName in body is required spec.customresourcedefinitions.owned.description in body is required

I added the spec.customresourcedefinitions.owned.displayName and spec.customresourcedefinitions.owned.description fields in this generated csv file, as below:

mac:memcached-operator jianzhang$ vim deploy/olm-catalog/memcached-operator/0.0.1/memcached-operator.v0.0.1.clusterserviceversion.yaml ... spec: apiservicedefinitions: {} customresourcedefinitions: owned: - kind: Memcached name: memcacheds.cache.example.com version: v1alpha1 description: Placeholder description displayName: Memcached Operator description: Placeholder description displayName: Memcached Operator ...

And then, it worked well, is it a bug? @hasbro17

mac:memcached-operator jianzhang$ oc create -f deploy/olm-catalog/memcached-operator/0.0.1/memcached-operator.v0.0.1.clusterserviceversion.yaml clusterserviceversion.operators.coreos.com/memcached-operator.v0.0.1 created mac:memcached-operator jianzhang$ operator-sdk version operator-sdk version: v0.7.0+git

@jianzhangbjz Yes it seems that might be a bug in the CSV generation. It's still seems to be the same behavior in v0.8.0.

@estroz Can you confirm if we don't generate spec.customresourcedefinitions.owned.displayName and spec.customresourcedefinitions.owned.description by default?
We should file an issue and try to fix this in the SDK. I can't recall if we were planning to parse these from CRD annotations, but if they're mandatory, as they seems to be, then we should always generate them with the placeholder values.

@bergerhoffer For now we might have to point out this fix in the getting-started guide for users generating their own CSVs. Essentially they just need to append the two fields fields mentioned above in their generated CSV manifests.
I'll update the getting-started guide until we get this fixed.

@jianzhangbjz we currently don't generate either of those fields, you'll have to add them yourself (see this issue for further discussion). However there is a PR open to fill those and other spec.customresourcedefinitions.owned fields.

Got it, thanks @hasbro17 @estroz

modules/managing-memcached-operator-using-olm.adoc

jianzhangbjz · 2019-05-13T06:27:18Z

modules/managing-memcached-operator-using-olm.adoc

+$ oc create -f deploy/crds/cache_v1alpha1_memcached_crd.yaml
+$ oc create -f deploy/service_account.yaml
+$ oc create -f deploy/role.yaml
+$ oc create -f deploy/role_binding.yaml


Actually, the csv object memcachedoperator.v0.0.1 still in Pending status after created the above objects. The Permissions are inappropriate. I submitted a PR to fix it. @hasbro17 Could you help have a review? Thanks!

mac:memcached-operator jianzhang$ oc describe csv memcachedoperator.v0.0.1 Name: memcachedoperator.v0.0.1 Namespace: default Labels: olm.api.7d4be2dfad49a2af=provided Annotations: kubectl.kubernetes.io/last-applied-configuration: {"apiVersion":"operators.coreos.com/v1alpha1","kind":"ClusterServiceVersion","metadata":{"annotations":{},"name":"memcachedoperator.v0.0.1... olm.operatorGroup: memcached-operator-group olm.operatorNamespace: default olm.targetNamespaces: default API Version: operators.coreos.com/v1alpha1 Kind: ClusterServiceVersion Metadata: Creation Timestamp: 2019-05-13T05:38:46Z Generation: 2 Resource Version: 59654 Self Link: /apis/operators.coreos.com/v1alpha1/namespaces/default/clusterserviceversions/memcachedoperator.v0.0.1 UID: 60a17d05-7541-11e9-bebe-0a5645815f04 Spec: Apiservicedefinitions: Customresourcedefinitions: Owned: Description: Represents a cluster of Memcached apps Display Name: Memcached App Kind: Memcached Name: memcacheds.cache.example.com Resources: Kind: Deployment Name: Version: v1beta2 Kind: ReplicaSet Name: Version: v1beta2 Kind: Pod Name: Version: v1 Spec Descriptors: Description: The desired number of member Pods for the deployment. Display Name: Size Path: size X - Descriptors: urn:alm:descriptor:com.tectonic.ui:podCount Status Descriptors: Description: The current status of the application. Display Name: Status Path: phase X - Descriptors: urn:alm:descriptor:io.kubernetes.phase Description: Explanation for the current status of the application. Display Name: Status Details Path: reason X - Descriptors: urn:alm:descriptor:io.kubernetes.phase:reason Version: v1alpha1 Description: Main enterprise application providing business critical features with high availabilty and no manual intervention. Display Name: Memcached Application Install: Spec: Deployments: Name: memcached-app-operator Spec: Replicas: 1 Selector: Match Labels: App: memcached-app-operator Template: Metadata: Labels: App: memcached-app-operator Spec: Containers: Command: /usr/local/bin/memcached-operator Image: quay.io/jzelinskie/memcached-operator:v0.0.1 Image Pull Policy: Always Name: sao Ports: Container Port: 8080 Protocol: TCP Restart Policy: Always Service Account: memcached-operator Service Account Name: memcached-operator Termination Grace Period Seconds: 5 Permissions: Rules: API Groups: * Resources: * Verbs: * Service Account Name: memcached-operator Strategy: deployment Install Modes: Supported: true Type: OwnNamespace Supported: true Type: SingleNamespace Supported: false Type: MultiNamespace Supported: false Type: AllNamespaces Keywords: memcached app Labels: Alm - Owner - Enterprise - App: memcached-app-operator Alm - Status - Descriptors: memcached-app-operator.v0.0.1 Maintainers: Email: [email protected] Name: Some Corp Maturity: alpha Provider: Name: Example URL: www.example.com Version: 0.0.1 Status: Certs Last Updated: <nil> Certs Rotate At: <nil> Conditions: Last Transition Time: 2019-05-13T05:38:46Z Last Update Time: 2019-05-13T05:38:46Z Message: requirements not yet checked Phase: Pending Reason: RequirementsUnknown Last Transition Time: 2019-05-13T05:38:46Z Last Update Time: 2019-05-13T05:38:46Z Message: one or more requirements couldn't be found Phase: Pending Reason: RequirementsNotMet Last Transition Time: 2019-05-13T05:38:46Z Last Update Time: 2019-05-13T05:38:46Z Message: one or more requirements couldn't be found Phase: Pending Reason: RequirementsNotMet Requirement Status: Group: operators.coreos.com Kind: ClusterServiceVersion Message: CSV missing minimum kube version specification Name: memcachedoperator.v0.0.1 Status: NotPresent Version: v1alpha1 Group: apiextensions.k8s.io Kind: CustomResourceDefinition Message: CRD is present and Established condition is true Name: memcacheds.cache.example.com Status: Present Uuid: c6c40b90-7540-11e9-bebe-0a5645815f04 Version: v1beta1 Dependents: Group: rbac.authorization.k8s.io Kind: PolicyRule Message: namespaced rule:{"verbs":["*"],"apiGroups":["*"],"resources":["*"]} Status: NotSatisfied Version: v1beta1 Group: Kind: ServiceAccount Message: Policy rule not satisfied for service account Name: memcached-operator Status: PresentNotSatisfied Version: v1 Events: Type Reason Age From Message ---- ------ ---- ---- ------- Normal RequirementsUnknown 13m operator-lifecycle-manager requirements not yet checked Normal RequirementsNotMet 13m (x2 over 13m) operator-lifecycle-manager one or more requirements couldn't be found

@jianzhangbjz Yes, the following rule was incorrect for a Role:

- apiGroups: - "*" ...

I've merged your PR. Were you able to verify the CSV with that fix?
If not then I can go through it again.

Sure, thanks!

bergerhoffer · 2019-05-13T14:34:24Z

Submitted updates for the comments I could take action on.

@jianzhangbjz @hasbro17 Let me know if the updates look okay, and whether any doc updates will be needed for the remaining two comments ([1] and [2]).

[1] #14592 (comment)
[2] #14592 (comment)

jianzhangbjz · 2019-05-20T09:59:07Z

@bergerhoffer Thanks for the update! @hasbro17 @lilic What do you think about the remaining comments?

bergerhoffer · 2019-05-23T20:19:48Z

@jianzhangbjz In speaking with @hasbro17, it sounds like there are a few things to follow up on, but that overall this PR fixes some issues. What do you say to merging this PR so that these updates get in for GA, and then we can open a ticket to follow up any additional updates that need to be made?

hasbro17 · 2019-05-23T20:38:51Z

I'm fine with merging the existing changes. The main workflow of using the predefined manifests should be fine. The generated manifests will have this hiccup that should be fairly easy/obvious to fix.

The issue with the missing CSV fields should be resolved once we fix that in the SDK.
Tracking this upstream operator-framework/getting-started#39

jianzhangbjz · 2019-05-24T05:33:38Z

@bergerhoffer It works for me to merge it, thanks!

lilic

Few nits but lgtm

lilic · 2019-05-24T13:54:25Z

modules/managing-memcached-operator-using-olm.adoc

+$ operator-sdk olm-catalog gen-csv --csv-version 0.0.1
+----
+
+NOTE: This command is run from the `memcached-operator/` directory that was created when you built the Memcached Operator.


Nit: I think the root directory of the project might be good to add.

@lilic Are you talking about $GOPATH/src/github.com/example-inc/memcached-operator/ like was used in the above section [1]?

[1] http://file.rdu.redhat.com/~ahoffer/2019/OSDOCS-391/applications/operator_sdk/osdk-getting-started.html#building-memcached-operator-using-osdk-osdk-getting-started

Due to time constraints, I'm going to merge this PR and can follow up in a separate PR if needed.

lilic · 2019-05-24T13:56:17Z

modules/managing-memcached-operator-using-olm.adoc

@@ -46,34 +52,56 @@ for more information on manually defining a manifest file.

 . *Deploy the Operator.*

-.. Deploy an Operator by applying the Operator’s manifest to the desired namespace
-in the cluster:
+.. Create an OperatorGroup that specifies the namespaces that the Operator will


Nit:

Suggested change

.. Create an OperatorGroup that specifies the namespaces that the Operator will

.. Create an `OperatorGroup` that specifies the namespaces that the operator will

Thanks for the suggestion but we actually do capitalize Operator and don't use markup as per our guidelines [1].

[1] https://github.com/openshift/openshift-docs/blob/enterprise-4.1/contributing_to_docs/doc_guidelines.adoc#api-object-formatting

bergerhoffer added peer-review-needed Signifies that the peer review team needs to review this PR branch/enterprise-4.1 labels Apr 25, 2019

bergerhoffer added this to the Future Release milestone Apr 25, 2019

openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Apr 25, 2019

bergerhoffer commented Apr 25, 2019

View reviewed changes

modules/managing-memcached-operator-using-olm.adoc Outdated Show resolved Hide resolved

hasbro17 approved these changes Apr 25, 2019

View reviewed changes

bergerhoffer force-pushed the OSDOCS-391 branch 2 times, most recently from 8f143be to 155f6cd Compare April 26, 2019 02:23

kalexand-rh self-requested a review April 26, 2019 20:33

kalexand-rh reviewed Apr 26, 2019

View reviewed changes

kalexand-rh added peer-review-done Signifies that the peer review team has reviewed this PR and removed peer-review-needed Signifies that the peer review team needs to review this PR labels Apr 26, 2019

bergerhoffer force-pushed the OSDOCS-391 branch 2 times, most recently from 9743b34 to 34e9c86 Compare April 29, 2019 20:20

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 12, 2019

bergerhoffer force-pushed the OSDOCS-391 branch from 34e9c86 to e69fa01 Compare May 12, 2019 19:17

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 12, 2019

jianzhangbjz suggested changes May 13, 2019

View reviewed changes

OSDOCS-391: Making a few OSDK updates

3cf1c2f

bergerhoffer force-pushed the OSDOCS-391 branch from e69fa01 to 3cf1c2f Compare May 13, 2019 14:30

lilic approved these changes May 24, 2019

View reviewed changes

bergerhoffer merged commit 119d383 into openshift:enterprise-4.1 May 24, 2019

vikram-redhat modified the milestones: Future Release, OCP 4.1 GA Jun 3, 2019

bergerhoffer deleted the OSDOCS-391 branch January 22, 2020 18:21

	.. Create an OperatorGroup that specifies the namespaces that the Operator will
	.. Create an `OperatorGroup` that specifies the namespaces that the operator will

OSDOCS-391: Making a few OSDK updates #14592

OSDOCS-391: Making a few OSDK updates #14592

Uh oh!

Conversation

bergerhoffer commented Apr 25, 2019

Uh oh!

bergerhoffer commented Apr 25, 2019

Uh oh!

Uh oh!

hasbro17 left a comment

Choose a reason for hiding this comment

Uh oh!

bergerhoffer commented Apr 26, 2019

Uh oh!

hasbro17 commented Apr 26, 2019

Uh oh!

kalexand-rh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bergerhoffer commented May 13, 2019

Uh oh!

jianzhangbjz commented May 20, 2019

Uh oh!

bergerhoffer commented May 23, 2019

Uh oh!

hasbro17 commented May 23, 2019

Uh oh!

jianzhangbjz commented May 24, 2019

Uh oh!

lilic left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!