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

operator-sdk/cmd: add up command #219

Merged

Conversation

fanminshi
Copy link
Contributor

@fanminshi fanminshi commented May 2, 2018

This pr adds a new command call up and its subcommand local.

Whenoperator-sdk up local is called, the operator is run locally on developer's machine with the access to a kubernetes cluster via the --kubeconfig flag.

ref: #142

cc/ @hasbro17 @spahl

@fanminshi
Copy link
Contributor Author

depends on #218

@fanminshi fanminshi closed this May 2, 2018
@fanminshi fanminshi reopened this May 2, 2018
@fanminshi fanminshi force-pushed the better_local_workflow branch 2 times, most recently from 52f101b to 6d50689 Compare May 2, 2018 23:23
@fanminshi fanminshi changed the title [WIP] operator-sdk/cmd: add up command operator-sdk/cmd: add up command May 2, 2018
@fanminshi
Copy link
Contributor Author

fanminshi commented May 2, 2018

manual test:

$ operator-sdk up -h
The operator-sdk up command launches the operator on the local machine
by building the operator binary with the ability to access a
kubernetes cluster using a Kubernete config file.

Usage:
  operator-sdk up [Flags] [flags]

Flags:
  -h, --help                help for up
      --kubeconfig string   The file path to kubernetes configuration file; defaults to $HOME/.kube/config


$ cat config/config.yaml
apiVersion: example.com/v1alpha1
kind: AppService
projectName: app-operator

$ pwd
/Users/fanminshi/work/src/github.com/operator-framework/app-operator
fanmins-MBP:app-operator fanminshi$ operator-sdk up
INFO[0000] Go Version: go1.10
INFO[0000] Go OS/Arch: darwin/amd64
INFO[0000] operator-sdk Version: 0.0.5+git
INFO[0000] starting appservices controller

@fanminshi fanminshi changed the title operator-sdk/cmd: add up command [WIP] operator-sdk/cmd: add up command May 3, 2018
@fanminshi
Copy link
Contributor Author

fanminshi commented May 3, 2018

In order to use up command, we need also need to create the CRD first. However, currently, the CRD and deployment manifests are tied together. Hence, I need to split that into two parts so that I can create CRD only.

ref: #76 #223

@spahl
Copy link
Contributor

spahl commented May 3, 2018

When I see a top level command called "up" I imagine that it is to deploy my operator to a running cluster. This seems to be for local development.

Is "up" the right command name here? Who is the user and what is his use case?

@fanminshi
Copy link
Contributor Author

Is "up" the right command here?
I am not sure what's the best naming scheme here. To me, "up" means up and running, and the idea of "up" to run the operator locally on your machine like rails server.

Who is the user and what is his use case?
Serval users have complained that the build and push image to registry model is kinda tedious for local development and testing. I hope this "up" command can solve this issue by let the user test their operator code immediately.

Maybe we can name this command with a better name to illustrate the intended behavior.

@spahl
Copy link
Contributor

spahl commented May 3, 2018

Maybe "up --local" ? That way "up" can be to run it inside the cluster? For dev and test it's probably valuable to be able to do both?

@fanminshi
Copy link
Contributor Author

Maybe "up --local" ? That way "up" can be to run it inside the cluster?

I am thinking about having separate sub commands for local and cluster deployment.
like up local [args] flags and up cluster [args] [flags] or r/up/deploy/ for better naming.

The reason for having separate command is that I suspect that up cluster might have a total of different flags and args than that of up local. If we combine the up cluster and the `up local into one command, I suspect most of the flags won't be apply to both and also may cause confusion to users.

@fanminshi
Copy link
Contributor Author

let's keep the design discussion here #142

@fanminshi fanminshi changed the title [WIP] operator-sdk/cmd: add up command operator-sdk/cmd: add up command May 4, 2018
@fanminshi
Copy link
Contributor Author

fanminshi commented May 4, 2018

operator-sdk CLI interface :

$ operator-sdk -h
A sdk for building operator with ease

Usage:
  operator-sdk [command]

Available Commands:
  build       Compiles code and builds artifacts
  generate    Invokes specific generator
  help        Help about any command
  new         Creates a new operator application
  up          Launches the operator

Flags:
  -h, --help   help for operator-sdk

Use "operator-sdk [command] --help" for more information about a command.

$ operator-sdk up -h
The up command has subcommands that can launch the operator in various ways.

Usage:
  operator-sdk up [command]

Available Commands:
  local       Launches the operator locally

Flags:
  -h, --help   help for up

Use "operator-sdk up [command] --help" for more information about a command.

$ operator-sdk up local -h
The operator-sdk up local command launches the operator on the local machine
by building the operator binary with the ability to access a
kubernetes cluster using a Kubernete config file.

Usage:
  operator-sdk up local [flags]

Flags:
  -h, --help                help for local
      --kubeconfig string   The file path to kubernetes configuration file; defaults to $HOME/.kube/config

$ pwd
/Users/fanminshi/work/src/github.com/operator-framework/app-operator

Local Test

$ kubectl get all
NAME             TYPE        CLUSTER-IP   EXTERNAL-IP   PORT(S)   AGE
svc/kubernetes   ClusterIP   10.96.0.1    <none>        443/TCP   43d

$ cat deploy/crd.yaml
apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
  name: apps.app.example.com
spec:
  group: app.example.com
  names:
    kind: App
    listKind: AppList
    plural: apps
    singular: app
  scope: Namespaced
  version: v1alpha1

$ kubectl create -f deploy/crd.yaml
customresourcedefinition "apps.app.example.com" created

$ operator-sdk up local
INFO[0000] Go Version: go1.10
INFO[0000] Go OS/Arch: darwin/amd64
INFO[0000] operator-sdk Version: 0.0.5+git
INFO[0000] starting apps controller

# In a different terminal
$ kubectl create -f deploy/cr.yaml
app "example" created
$ kubectl get po
NAME       READY     STATUS    RESTARTS   AGE
busy-box   1/1       Running   0          3s

@fanminshi
Copy link
Contributor Author

Reflected the discussion from #142 in this pr.
PTAL cc/ @hasbro17 @spahl

Short: "Launches the operator locally",
Long: `The operator-sdk up local command launches the operator on the local machine
by building the operator binary with the ability to access a
kubernetes cluster using a Kubernete config file.
Copy link
Contributor

Choose a reason for hiding this comment

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

using a Kubernete config file ==> using a kubeconfig file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed!


cmdError "github.com/operator-framework/operator-sdk/commands/operator-sdk/error"
"github.com/operator-framework/operator-sdk/pkg/generator"
yaml "gopkg.in/yaml.v2"
Copy link
Contributor

Choose a reason for hiding this comment

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

Leave a line break before this.


_, err := os.Stat(kubeConfig)
if err != nil && os.IsNotExist(err) {
cmdError.ExitWithError(cmdError.ExitError, fmt.Errorf("failed to fine the Kubernetes config file (%v): %v", kubeConfig, err))
Copy link
Contributor

Choose a reason for hiding this comment

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

failed to fine the Kubernetes config file ==> failed to find the kubeconfig file

dc := exec.Command(gocmd, run, filepath.Join(cmd, projectName, main))
dc.Stdout = os.Stdout
dc.Stderr = os.Stderr
dc.Env = append(os.Environ(), fmt.Sprintf("KUBERNETES_CONFIG=%v", kubeConfig))
Copy link
Contributor

Choose a reason for hiding this comment

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

Make the string KUBERNETES_CONFIG a constant because it's common to pkg/k8sclient/client.go and here.

}
}

func up(projectName string) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you rename this to upLocal(). Because we'll be adding an up cluster command in the same package later on.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah sure!

@fanminshi fanminshi force-pushed the better_local_workflow branch 2 times, most recently from 96c24bc to 32b397e Compare May 4, 2018 22:36
@fanminshi
Copy link
Contributor Author

all fixed PTAL cc/ @hasbro17

@hasbro17
Copy link
Contributor

hasbro17 commented May 4, 2018

LGTM

@fanminshi fanminshi merged commit 3359d42 into operator-framework:master May 4, 2018
@fanminshi fanminshi mentioned this pull request May 4, 2018
2 tasks
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.

3 participants