Switch over to new CLI and deprecate `operator-sdk new --type=go` by hasbro17 · Pull Request #3190 · operator-framework/operator-sdk

hasbro17 · 2020-06-08T07:47:45Z

Make the new Kubebuilder aligned CLI the default for Go operators.

There are 3 main changes/commits in this PR to make it easier to review:

Make the new Go docs visible and mark the old ones as legacy:
- Old/Legacy docs moved website/content/en/docs/golang/*=> website/content/en/docs/golang/legacy/*
- New docs moved website/content/en/docs/kubebuilder/*=> website/content/en/docs/golang/new/*
- Links updated in other docs accordingly
- Link titles and weights added to the new docs to make them visible
Generate new CLI docs:
- Updated hack/generate/cli-doc/gen-cli-doc.go to generate both the legacy and new CLI reference docs
- Generated new CLI reference docs at website/content/en/docs/new-cli/
- Old CLI docs can't be moved to not break links in CHANGELOG notes but marked as legacy in its _index.md.
Deprecate operator-sdk new --type=go and make the new KB CLI the default entrypoint outside of legacy projects:
- main.go defaults to the new KB CLI except when running in legacy Go, or Ansible/Helm projects.
- operator-sdk new --type=go has been removed so legacy Go projects cannot be scaffolded anymore. --type=Ansible/Helm still exist.
- All commands in legacy Go projects will print out a Deprecation Notice and point to the new CLI and layout.

TODO:

Rebase once missing reference docs are added to the new docs in Add and align reference docs for new KB docs #3209
Inspect the rendered website docs to make sure all docs are visible with the correct link weights
Update/Disable test scripts using operator-sdk new --type=Go

Follow up PRs:

Remove internal Go scaffolding code
Reshuffle docs to de-emphasize legacy Go docs by nesting them inside the new ones.
Add CHANGELOG fragment for v0.19.0 to summarize removal of new --type=Go and deprecation of old CLI.
Add note to the new quickstart guide on installing control-plane binaries (etcd, kube-apiserver etc) for envtest to run correctly.

joelanford

Can we also remove all of the now-unused Go scaffolds?

hasbro17 · 2020-06-08T19:18:01Z

Can we also remove all of the now-unused Go scaffolds?

@joelanford I intentionally left that out of this PR so reviewers could focus on verifying the more important docs, links and CLI changes.
I can add a TODO and do a follow up PR for removing the unused scaffold code once this gets merged.

Or since it's just straight up removing code I can tack it on at the very end if the PR size isn't too large.

hasbro17 · 2020-06-08T19:21:42Z

Something's off with the doc links. Going to fix that.
https://travis-ci.org/github/operator-framework/operator-sdk/jobs/695909825#L327

- /target/docs/contribution-guidelines/testing/travis-build/index.html
  *  internally linking to ../../../golang/quickstart#a-note-on-dependency-management, which does not exist (line 1113)
     <a href="../../../golang/quickstart#a-note-on-dependency-management">note</a>
  *  linking to internal hash #a-note-on-dependency-management that does not exist (line 1113)
     <a href="../../../golang/quickstart#a-note-on-dependency-management">note</a>
- /target/docs/faq/index.html
  *  internally linking to /docs/golang/legacy/legacy/monitoring/prometheus/#garbage-collection, which does not exist (line 993)
...

joelanford · 2020-06-08T20:54:18Z

Can we also remove all of the now-unused Go scaffolds?

@joelanford I intentionally left that out of this PR so reviewers could focus on verifying the more important docs, links and CLI changes.
I can add a TODO and do a follow up PR for removing the unused scaffold code once this gets merged.

Or since it's just straight up removing code I can tack it on at the very end if the PR size isn't too large.

Either way seems fine to me. Just wanted to make sure that is covered.

camilamacedo86 · 2020-06-09T11:22:01Z

Hi @hasbro17,

It s not passing in the tests. Shows that it missing update the links reference. Maybe can be easy to split the PR one for the docs and other for the code source.

Regards the code see:

cmd/operator-sdk/new/cmd.go:385:6: func `getDeps` is unused (unused)
cmd/operator-sdk/new/cmd.go:414:6: func `validateProject` is unused (unused)

Regards the docs, also see that it shows not accurate. It has now 2 installations and is missing the Golang menu. See:

jmccormick2001

Create a validating or mutating Admission Webhook

I get a 404 on that link in the docs.

jmccormick2001 · 2020-06-09T14:43:18Z

Also see the advanced topics doc for more use cases and under the hood details.

I get a 404 on that link in the docs when served locally.

jmccormick2001 · 2020-06-09T14:44:08Z

Filter watch events using predicates

I get a 404 on that link in the docs when served locally.

jmccormick2001 · 2020-06-09T14:48:49Z

completely unrelated to this PR, but I found it interesting that no labels were added to the role/sa/binding resources whereby you could remove them in an explicit way using a label selector:

jeffmc@breve:/projects/memcached-operator$ kubectl get role --show-labels
NAME CREATED AT LABELS
memcached-operator-leader-election-role 2020-06-09T14:40:14Z
jeffmc@breve:/projects/memcached-operator$ kubectl get role,rolebinding,sa --show-labels
NAME CREATED AT LABELS
role.rbac.authorization.k8s.io/memcached-operator-leader-election-role 2020-06-09T14:40:14Z

NAME ROLE AGE LABELS
rolebinding.rbac.authorization.k8s.io/memcached-operator-leader-election-rolebinding Role/memcached-operator-leader-election-role 26s

NAME SECRETS AGE LABELS
serviceaccount/default 1 27d

so, the line in the docs under cleanup:

$ kubectl delete role,rolebinding --all

could be sort of dangerous if you have other stuff running in the default namespace. Would be great if a unique label were applied, maybe the operator name or similar.

hasbro17 · 2020-06-09T17:29:03Z

The broken links are due to a missing index file. Will push that fix along with other changes in a bit.

Maybe can be easy to split the PR one for the docs and other for the code source.

@camilamacedo86 I'd rather not split this up if possible because the CLI changes and the doc changes depend on each other.
Otherwise we could end up with deprecated docs on the master and the CLI won't reflect those changes.

hasbro17 · 2020-06-15T02:29:55Z

So I was going to disable the old e2e test for Go operators completely now that we're using the new CLI (which already has an e2e test).

But I think we probably could keep it around even though the old CLI for Go projects is deprecated, it's not unsupported in existing projects.
So most likely if I just update operator-sdk new in the go scaffolding scripts to use operator-sdk v0.18.1 the rest of the old CLI can still be tested in an old project layout.

operator-sdk/hack/tests/scaffolding/scaffold-memcached.go

Lines 62 to 70 in 6e000a4

    
           log.Print("Creating new operator project") 
        
           cmdOut, err := exec.Command("operator-sdk", 
        
           	"new", 
        
           	operatorName, 
        
           	"--repo", testRepo, 
        
           	"--skip-validation").CombinedOutput() 
        
           if err != nil { 
        
           	log.Fatalf("Error: %v\nCommand Output: %s\n", err, string(cmdOut)) 
        
           }

hasbro17 · 2020-06-15T02:32:26Z

The docs and CLI part of this PR should be ready for reviews (based on the layout in #3190 (comment)).

I'll fix the old Go e2e test in the meanwhile.

joelanford

/lgtm

Great job!

joelanford · 2020-06-16T03:19:53Z

+TODO: Once versioned docs are supported on the website the CHANGELOG notes
+that link to the legacy CLI should be updated before being removed on the master
+branch.


@asmacdo FYI

joelanford · 2020-06-16T03:22:49Z

 weight: 2
 ---

+**Note:** This guide is for the legacy CLI and project layout. See the [new docs][new_docs] for the [Kubebuilder aligned CLI][new_CLI] and project layout.


Given that this quickstart assumes operator-sdk new --type=go exists, should we remove it?

This is non-blocking and could be done in a follow-up.

Oh yeah forgot about that.
There's some stuff in the FAQ talking about metrics as well.
I'll add a TODO to follow this up.

estroz

One nit otherwise

/lgtm

camilamacedo86 · 2020-06-16T21:49:10Z

+	new.NewCmd(),

 	alpha.NewCmd(),
 	build.NewCmd(),


The operator-sdk create api is not showing when we use the help. Not a blocker too. We can do it in a follow up too.

That's a plugin issue right @estroz
You have to be inside an initialized project for that help to show up for the correct plugin version I believe.

$ operator-sdk create api -h Scaffold a Kubernetes API. For project-specific information, run this command in the root directory of a project. Note: unable to find configuration file, project must be initialized Usage: operator-sdk create api [flags] Flags: -h, --help help for api

And inside a project:

$ operator-sdk create api -h Scaffold a Kubernetes API by creating a Resource definition and / or a Controller. create resource will prompt the user for if it should scaffold the Resource and / or Controller. To only scaffold a Controller for an existing Resource, select "n" for Resource. To only define the schema for a Resource without writing a Controller, select "n" for Controller. After the scaffold is written, api will run make on the project. Usage: operator-sdk create api [flags] Examples: # Create a frigates API with Group: ship, Version: v1beta1 and Kind: Frigate operator-sdk create api --group ship --version v1beta1 --kind Frigate ...

You need to set --project-version to output help for the default plugin. However I do see create listed in the available subcommands when I run operator-sdk -h regardless of where the binary is.

I saw: create Scaffold a Kubernetes API or webhook 👍

camilamacedo86

/lgtm
/approve

terrific work @hasbro17
I added 2 comments which IMO are not blocker and we can check in a follow-up.

hasbro17 · 2020-06-16T23:47:23Z

Merging this. Will address the TODOs(in the description) for the CHANGELOG, docs layout reshuffle and Go scaffold removal in follow up PRs.

balasgit · 2020-07-22T22:10:43Z

Any inputs "how to use the new CLI" for the operator based on GO ? any samples or docs ? thanks

estroz · 2020-07-22T22:30:39Z

@balasgit https://sdk.operatorframework.io/docs/golang/quickstart/

hasbro17 added the kubebuilder-integration Relates to rewriting the SDK in Kubebuilder plugin form label Jun 8, 2020

hasbro17 requested review from asmacdo, camilamacedo86, estroz, jmccormick2001, jmrodri, joelanford and varshaprasad96 June 8, 2020 07:47

openshift-ci-robot requested review from fabianvf and shawn-hurley June 8, 2020 07:47

hasbro17 removed the request for review from shawn-hurley June 8, 2020 07:54

camilamacedo86 reviewed Jun 8, 2020

View reviewed changes

Comment thread website/content/en/docs/golang/kubebuilder/crds-scope.md Outdated

joelanford reviewed Jun 8, 2020

View reviewed changes

Comment thread cmd/operator-sdk/new/cmd.go Outdated

Comment thread website/content/en/docs/golang/kubebuilder/crds-scope.md Outdated

jmccormick2001 reviewed Jun 9, 2020

View reviewed changes

estroz reviewed Jun 9, 2020

View reviewed changes

Comment thread cmd/operator-sdk/main.go Outdated

hasbro17 changed the title ~~Switch over to new CLI and deprecate operator-sdk new --type=go~~ WIP: Switch over to new CLI and deprecate operator-sdk new --type=go Jun 9, 2020

openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 9, 2020

hasbro17 mentioned this pull request Jun 10, 2020

Add and align reference docs for new KB docs #3209

Merged

hasbro17 force-pushed the default-to-new-CLI-and-docs branch 4 times, most recently from 756034f to eb17e7b Compare June 10, 2020 01:56

hasbro17 changed the title ~~Switch over to new CLI and deprecate operator-sdk new --type=go~~ WIP: Switch over to new CLI and deprecate operator-sdk new --type=go Jun 15, 2020

openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 15, 2020

hasbro17 changed the title ~~WIP: Switch over to new CLI and deprecate operator-sdk new --type=go~~ Switch over to new CLI and deprecate operator-sdk new --type=go Jun 15, 2020

openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 15, 2020

hasbro17 requested review from camilamacedo86, estroz, jmccormick2001 and joelanford June 15, 2020 02:32

e2e-go-scaffold uses old CLI binary for operator-sdk new

7fad2a4

joelanford approved these changes Jun 16, 2020

View reviewed changes

openshift-ci-robot assigned joelanford Jun 16, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 16, 2020

joelanford mentioned this pull request Jun 16, 2020

docs: section about dropped openapi option #3243

Closed

estroz approved these changes Jun 16, 2020

View reviewed changes

Comment thread hack/tests/scaffolding/e2e-go-scaffold.sh

openshift-ci-robot assigned estroz Jun 16, 2020

camilamacedo86 reviewed Jun 16, 2020

View reviewed changes

Comment thread cmd/operator-sdk/cli/cli.go

camilamacedo86 reviewed Jun 16, 2020

View reviewed changes

camilamacedo86 approved these changes Jun 16, 2020

View reviewed changes

openshift-ci-robot assigned camilamacedo86 Jun 16, 2020

hasbro17 merged commit e128b9e into operator-framework:master Jun 16, 2020

hasbro17 deleted the default-to-new-CLI-and-docs branch June 17, 2020 00:51

This was referenced Jun 17, 2020

Nest legacy Go docs under new ones #3248

Merged

Remove unused Go operator scaffold code #3249

Merged

Add CHANGELOG fragments for new CLI #3254

Merged

camilamacedo86 mentioned this pull request Jul 22, 2020

can't use "operator-sdk new" create go type operator #3485

Closed

Conversation

hasbro17 commented Jun 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

joelanford left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

hasbro17 commented Jun 8, 2020

Uh oh!

hasbro17 commented Jun 8, 2020

Uh oh!

joelanford commented Jun 8, 2020

Uh oh!

camilamacedo86 commented Jun 9, 2020

Uh oh!

jmccormick2001 left a comment

Choose a reason for hiding this comment

Uh oh!

jmccormick2001 commented Jun 9, 2020

Uh oh!

jmccormick2001 commented Jun 9, 2020

Uh oh!

jmccormick2001 commented Jun 9, 2020

Uh oh!

Uh oh!

hasbro17 commented Jun 9, 2020

Uh oh!

hasbro17 commented Jun 15, 2020

Uh oh!

hasbro17 commented Jun 15, 2020

Uh oh!

joelanford left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

joelanford Jun 16, 2020

Choose a reason for hiding this comment

Uh oh!

joelanford Jun 16, 2020

Choose a reason for hiding this comment

Uh oh!

hasbro17 Jun 16, 2020

Choose a reason for hiding this comment

Uh oh!

estroz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

camilamacedo86 Jun 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hasbro17 Jun 16, 2020

Choose a reason for hiding this comment

Uh oh!

estroz Jun 16, 2020

Choose a reason for hiding this comment

Uh oh!

camilamacedo86 Jun 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

camilamacedo86 left a comment

Choose a reason for hiding this comment

Uh oh!

hasbro17 commented Jun 16, 2020

Uh oh!

balasgit commented Jul 22, 2020

Uh oh!

estroz commented Jul 22, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

hasbro17 commented Jun 8, 2020 •

edited

Loading

camilamacedo86 Jun 16, 2020 •

edited

Loading

camilamacedo86 Jun 17, 2020 •

edited

Loading