BZ1886810: Updated operator-sdk appendices to have updated types #36938

snarayan-redhat · 2021-09-30T09:48:28Z

Bug: https://bugzilla.redhat.com/show_bug.cgi?id=1886810

Preview link: https://deploy-preview-36938--osdocs.netlify.app/openshift-enterprise/latest/operators/operator_sdk/osdk-appendices.html#osdk-project-scaffolding-layout-go_operator-appendices

@tbuskey / @emmajiafan - Kindly help with QE review.
@jmrodri - Kindly help with SME review.

netlify · 2021-09-30T09:51:00Z

✔️ Deploy Preview for osdocs ready!

🔨 Explore the source changes: 14f9258

🔍 Inspect the deploy log: https://app.netlify.com/sites/osdocs/deploys/616448700fbe350007648105

😎 Browse the preview: https://deploy-preview-36938--osdocs.netlify.app

ctauchen

Thanks for this bug fix. I've made a few copyedits for you to consider.

FYI @bergerhoffer @bobfuru

ctauchen · 2021-10-04T12:00:48Z

modules/osdk-project-staffolding-layout.adoc

Suggested change

|Contains the helm chart used while creating the project.

|Contains the Helm chart used while creating the project.

The directories for the helm operator are also out of date.

Using this command operator-sdk init --domain example.com --plugins helm the directory structure looks like this:

tree -d . ├── config │ ├── default │ ├── manager │ ├── manifests │ ├── prometheus │ ├── rbac │ └── scorecard │ ├── bases │ └── patches └── helm-charts

And with an API added, using operator-sdk create api --group demo --version v1alpha1 --kind Nginx the structure looks like this:

$ tree -d . ├── config │ ├── crd │ │ └── bases │ ├── default │ ├── manager │ ├── manifests │ ├── prometheus │ ├── rbac │ ├── samples │ └── scorecard │ ├── bases │ └── patches └── helm-charts └── nginx └── templates └── tests 16 directories

You can see the quickstart if you want to play with it some. https://sdk.operatorframework.io/docs/building-operators/helm/quickstart/

ctauchen · 2021-10-04T12:02:34Z

modules/osdk-project-staffolding-layout.adoc

s/by/in
What do you think?
Unless 'scenario' in this context is jargon, and can be used as the subject of 'use'.

Currently this piece is commented deliberately, so that SME can take a look and suggest whether the description needs to be at a subfolder level

And scenario is a jargon here, will re-verify with SME on this and make necessary changes.

ctauchen · 2021-10-04T12:04:01Z

modules/osdk-project-staffolding-layout.adoc

Suggested change

// |Contains files that run a full end-to-end test of your operator. It does not require an existing cluster or external registry, and can run in CI environments that allow users to run privileged containers, for example, Travis.

// |Contains files that run a full end-to-end test of your Operator. It does not require an existing cluster or external registry, and can run in CI environments that allow users to run privileged containers, such as Travis.

It does not require ...

The antecedent of It is not clear. Perhaps you mean The test?

Noted the comment. Will implement once I get a feedback from SME.

ctauchen · 2021-10-04T12:05:13Z

modules/osdk-project-staffolding-layout.adoc

Suggested change

// |Contains files that is used to run an end-to-end test of your operator against an existing cluster. The operator image needs to be available to the cluster.

// |Contains files that are used to run an end-to-end test of your Operator against an existing cluster. The Operator image needs to be available to the cluster.

The Operator image needs to be available to the cluster.

You discuss two Operators, and it's not immediately clear which Operator this sentence refers to.

ctauchen · 2021-10-04T12:05:32Z

modules/osdk-project-staffolding-layout.adoc

Suggested change

|Contains the files that are used for testing the ansible roles.

|Contains the files that are used for testing the Ansible roles.

bergerhoffer

No additional comments other than what @ctauchen already pointed out. But just wanted to quickly confirm that this change is indeed only needed in 4.6, and not in anything 4.7 or newer

bobfuru · 2021-10-04T18:06:40Z

I also concur with @ctauchen's feedback. I checked and it does make sense that this is for 4.6 only, as the module was removed beginning in 4.7.

tbuskey · 2021-10-05T12:24:27Z

/lgtm

jmrodri · 2021-10-09T23:19:50Z

modules/osdk-project-staffolding-layout.adoc

Suggested change

Ansible-based Operator projects generated using the `operator-sdk new --type ansible` command contain the following directories and files:

Ansible-based Operator projects generated using the `operator-sdk init --plugins ansible` command contain the following directories and files:

The new subcommand was removed.

The list of directories in this PR is old. There are a set of new ones now.
Here are the default list of directories (only need to worry about the top part). I used the following command to create it operator-sdk init --domain example.com --plugins ansible

. ├── config │ ├── default │ ├── manager │ ├── manifests │ ├── prometheus │ ├── rbac │ ├── scorecard │ │ ├── bases │ │ └── patches │ └── testing │ └── pull_policy ├── molecule │ ├── default │ └── kind ├── playbooks └── roles 16 directories

With an API, operator-sdk create api --group cache --version v1alpha1 --kind Memcached --generate-role present the structure looks like this:

$ tree -d . ├── config │ ├── crd │ │ └── bases │ ├── default │ ├── manager │ ├── manifests │ ├── prometheus │ ├── rbac │ ├── samples │ ├── scorecard │ │ ├── bases │ │ └── patches │ └── testing │ └── pull_policy ├── molecule │ ├── default │ │ └── tasks │ └── kind ├── playbooks └── roles └── memcached ├── defaults ├── files ├── handlers ├── meta ├── tasks ├── templates └── vars 28 directories

If you need to experiment feel free to check out the quickstart upstream doc: https://sdk.operatorframework.io/docs/building-operators/ansible/quickstart/

jmrodri · 2021-10-09T23:33:43Z

modules/osdk-project-staffolding-layout.adoc

The directories for the helm operator are also out of date.

Using this command operator-sdk init --domain example.com --plugins helm the directory structure looks like this:

tree -d . ├── config │ ├── default │ ├── manager │ ├── manifests │ ├── prometheus │ ├── rbac │ └── scorecard │ ├── bases │ └── patches └── helm-charts

And with an API added, using operator-sdk create api --group demo --version v1alpha1 --kind Nginx the structure looks like this:

$ tree -d . ├── config │ ├── crd │ │ └── bases │ ├── default │ ├── manager │ ├── manifests │ ├── prometheus │ ├── rbac │ ├── samples │ └── scorecard │ ├── bases │ └── patches └── helm-charts └── nginx └── templates └── tests 16 directories

You can see the quickstart if you want to play with it some. https://sdk.operatorframework.io/docs/building-operators/helm/quickstart/

jmrodri

/lgtm

I did not realize this was for OpenShift 4.6. My comments from #36938 (review) are not valid against 4.6 they are for 4.8 and beyond.

openshift-ci · 2021-10-11T14:20:16Z

New changes are detected. LGTM label has been removed.

BZ1889810: Added Ansible-based information Added Ansible-based information Added Ansible-based information Added Ansible-based information Implemented review comments Removed commented content

openshift-ci bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Sep 30, 2021

snarayan-redhat force-pushed the BZ1886810_edit_osdk_flags branch from f343d2c to 6418d7a Compare September 30, 2021 09:56

ctauchen reviewed Oct 4, 2021

View reviewed changes

bergerhoffer added branch/enterprise-4.6 peer-review-done Signifies that the peer review team has reviewed this PR labels Oct 4, 2021

bergerhoffer added this to the Next Release milestone Oct 4, 2021

bergerhoffer reviewed Oct 4, 2021

View reviewed changes

openshift-ci bot assigned tbuskey Oct 5, 2021

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 5, 2021

jmrodri suggested changes Oct 9, 2021

View reviewed changes

jmrodri approved these changes Oct 9, 2021

View reviewed changes

openshift-ci bot assigned jmrodri Oct 9, 2021

jmrodri removed their assignment Oct 10, 2021

snarayan-redhat force-pushed the BZ1886810_edit_osdk_flags branch from 6418d7a to 997bb45 Compare October 11, 2021 14:20

openshift-ci bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 11, 2021

BZ1889810: Added Ansible-based information

14f9258

BZ1889810: Added Ansible-based information Added Ansible-based information Added Ansible-based information Added Ansible-based information Implemented review comments Removed commented content

snarayan-redhat force-pushed the BZ1886810_edit_osdk_flags branch from 997bb45 to 14f9258 Compare October 11, 2021 14:21

bobfuru merged commit ecdf46a into openshift:enterprise-4.6 Oct 11, 2021

	\|Contains the helm chart used while creating the project.
	\|Contains the Helm chart used while creating the project.

	// \|Contains files that run a full end-to-end test of your operator. It does not require an existing cluster or external registry, and can run in CI environments that allow users to run privileged containers, for example, Travis.
	// \|Contains files that run a full end-to-end test of your Operator. It does not require an existing cluster or external registry, and can run in CI environments that allow users to run privileged containers, such as Travis.

	// \|Contains files that is used to run an end-to-end test of your operator against an existing cluster. The operator image needs to be available to the cluster.
	// \|Contains files that are used to run an end-to-end test of your Operator against an existing cluster. The Operator image needs to be available to the cluster.

	\|Contains the files that are used for testing the ansible roles.
	\|Contains the files that are used for testing the Ansible roles.

	Ansible-based Operator projects generated using the `operator-sdk new --type ansible` command contain the following directories and files:
	Ansible-based Operator projects generated using the `operator-sdk init --plugins ansible` command contain the following directories and files:

BZ1886810: Updated operator-sdk appendices to have updated types #36938

BZ1886810: Updated operator-sdk appendices to have updated types #36938

Uh oh!

Conversation

snarayan-redhat commented Sep 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

netlify bot commented Sep 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ctauchen 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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bergerhoffer left a comment

Choose a reason for hiding this comment

Uh oh!

bobfuru commented Oct 4, 2021

Uh oh!

tbuskey commented Oct 5, 2021

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmrodri left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci bot commented Oct 11, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

snarayan-redhat commented Sep 30, 2021 •

edited

Loading

netlify bot commented Sep 30, 2021 •

edited

Loading