Skip to content

docs: section about dropped openapi option#3243

Closed
georgettica wants to merge 6 commits intooperator-framework:masterfrom
georgettica:patch-1
Closed

docs: section about dropped openapi option#3243
georgettica wants to merge 6 commits intooperator-framework:masterfrom
georgettica:patch-1

Conversation

@georgettica
Copy link
Copy Markdown

Because operator-sdk generate openapi has been dropped, it is good to add here a section on how to work with openapi from now on

Description of the change:

Motivation for the change:

Because `operator-sdk generate openapi` has been dropped, it is good to add here a section on how to work with openapi from now on
@georgettica
Copy link
Copy Markdown
Author

@camilamacedo86 @joelanford I created this PR

Comment thread website/content/en/docs/faq.md Outdated
Comment thread website/content/en/docs/faq.md Outdated
Comment thread website/content/en/docs/faq.md Outdated
Copy link
Copy Markdown
Contributor

@camilamacedo86 camilamacedo86 left a comment

Choose a reason for hiding this comment

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

See the comments to get a few suggestions.

Copy link
Copy Markdown
Contributor

@camilamacedo86 camilamacedo86 left a comment

Choose a reason for hiding this comment

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

few nits, otherwise it is great.
Really tks for the collab with 🥇

georgettica and others added 4 commits June 16, 2020 15:13
Co-authored-by: Camila Macedo <cmacedo@redhat.com>
Co-authored-by: Camila Macedo <cmacedo@redhat.com>
Co-authored-by: Camila Macedo <cmacedo@redhat.com>
@georgettica
Copy link
Copy Markdown
Author

did all the changes you requested @camilamacedo86, can you take a look?

Copy link
Copy Markdown
Member

@joelanford joelanford left a comment

Choose a reason for hiding this comment

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

Just some suggestions to make this more standalone and future-proof.

Comment thread website/content/en/docs/faq.md Outdated
## I cannot use the command `operator-sdk generate openapi`
The command `operator-sdk generate openapi` was removed from the SDK tool in the version `v0.17.0`. It is now recommended to use [openapi-gen][openapi-gen] directly for OpenAPI code generation.

Note that in the example on the docs, the flag `-h ./hack/boilerplate.go.txt` is used to allow add the LICENCE comment on the top of the documents and then, it can be removed.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Where is the example? I think we should embed that directly in the FAQ.

Note that we're switching to the kubebuilder-based project layout in #3190, so we should make the example work for that project structure (not our existing project structure).

@hasbro17 did you run across an examples in our docs for running openapi-gen that had to change due to the new layout?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@joelanford Hmm no. The only place where we mention how to use openapi-gen is in the version upgrade guide right around the time we dropped support for operator-sdk generate openapi.
https://sdk.operatorframework.io/docs/migration/version-upgrade-guide/#v017x

But I guess for the new project layout you'd just need to update the input/output package paths to your API Go types and you can use that:

# Install openapi-gen binary. Although not this way since this pollutes your SDK's go.mod file
$ go get k8s.io/kube-openapi/cmd/openapi-gen

$ tree apis/
apis/
└── cache
    └── v1alpha1
        ├── groupversion_info.go
        ├── memcached_types.go
        └── zz_generated.deepcopy.go

# openapi-gen always needs a go header file so just use an empty one if not needed
$ touch header.txt

$ openapi-gen  --output-base="" --input-dirs=./apis/cache/v1alpha1 --output-package=./apis/cache/v1alpha1 --output-file-base=zz_generated.openapi -h=./header.txt

$ tree apis/
apis/
└── cache
    └── v1alpha1
        ├── groupversion_info.go
        ├── memcached_types.go
        ├── zz_generated.deepcopy.go
        └── zz_generated.openapi.go

Oh also, with Kubebuilder we don't have the // +k8s:openapi-gen=true tags in our <kind>_type.go scaffolds anymore. So you'd probably need to add those to the Go types yourself.

Copy link
Copy Markdown
Contributor

@camilamacedo86 camilamacedo86 Jun 19, 2020

Choose a reason for hiding this comment

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

Hi @georgettica could you please check it with the new layout to provide the example? To do that you need use the master branch and run mkdir <projectname>, cd <projectname> and then, operator-sdk init. It will scaffold the project with the new layout.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

will it skaffold a current project with the new information? for I got a confusing result.
I ran the command on my current operator and recieved an error but files and folders were created.

When I ran the init command a hack/boilerplate.go.txt was created so if I were using the command in https://sdk.operatorframework.io/docs/migration/version-upgrade-guide/#v017x

and I guess the tags// +k8s:openapi-gen=true were removed for extracting the openapi-gen from the codebase, so a not should refer to that

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hi @georgettica,

SDK is in a process to be integrated with KB which means that its project layouts will be aligned. More info : Integrating Kubebuilder and Operator SDK. If you run operator-sdk init you will scaffold a project with the new layout.

What is missing here is we provide the example and the info let the users know how to do with the Legacy layout and the new one. Is it make sense?

Copy link
Copy Markdown
Contributor

@camilamacedo86 camilamacedo86 left a comment

Choose a reason for hiding this comment

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

Missing:

  • Address comment/suggestion to reword
  • Check and provide the solution/example with the new layout.

Co-authored-by: Joe Lanford <joe.lanford@gmail.com>
@camilamacedo86
Copy link
Copy Markdown
Contributor

Hi @georgettica,

I am closing this one since it is open for a while without address the suggestion.
However, feel free to re-open this one with you wish and if you be able to address the requests changes to add info requested.

Really thank you for your contribution.
And please you are more than welcome to push PR's and contribute to this project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants