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

commands/operator-sdk/cmd/local: add namespace flag #274

Merged
merged 2 commits into from
May 23, 2018

Conversation

jhernandezb
Copy link
Contributor

@jhernandezb jhernandezb commented May 22, 2018

ref: #273

This adds the --namespace flag to the up local command and the default value is the "default" namespace.

Example:

operator-sdk up local --namespace operator

Help output

$ operator-sdk up local --help
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 kubeconfig 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
      --namespace string        The namespace where the operator watches for changes. (default "default")
      --operator-flags string   The flags that the operator needs. Example: "--flag1 value1 --flag2=value2"

@@ -28,13 +28,15 @@ kubernetes cluster using a kubeconfig file.

upLocalCmd.Flags().StringVar(&kubeConfig, "kubeconfig", "", "The file path to kubernetes configuration file; defaults to $HOME/.kube/config")
upLocalCmd.Flags().StringVar(&operatorFlags, "operator-flags", "", "The flags that the operator needs. Example: \"--flag1 value1 --flag2=value2\"")
upLocalCmd.Flags().StringVarP(&namespace, "namespace", "n", "default", "The namespace where the operator will watch.")
Copy link
Contributor

Choose a reason for hiding this comment

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

I am debating whether should we add a short named flag for namespace because none of the other flags have a short version.

I'd suggest we don't add it now. Maybe we can revisit adding the short flags in a future pr.

Suggest to avoid using the word will and keep description in present tense: The namespace where the operator will watch. -> The namespace where the operator watches for changes?

@fanminshi
Copy link
Contributor

Could you also update the https://github.com/operator-framework/operator-sdk/blob/master/CHANGELOG.md to include this change?

Just modify Added operator-sdk up command ... to incorporate this change.

@jhernandezb
Copy link
Contributor Author

I've applied the changes please take a look.

CHANGELOG.md Outdated
@@ -2,7 +2,7 @@

### Added

- Added `operator-sdk up` command to help deploy an operator. Currently supports running an operator locally against an existing cluster e.g `operator-sdk up local --kubeconfig=<path-to-kubeconfig>`. See `operator-sdk up -h` for help. [#219](https://github.com/operator-framework/operator-sdk/pull/219)
- Added `operator-sdk up` command to help deploy an operator. Currently supports running an operator locally against an existing cluster e.g `operator-sdk up local --kubeconfig=<path-to-kubeconfig> --namespace=<operator-namespace>`. See `operator-sdk up -h` for help. [#219](https://github.com/operator-framework/operator-sdk/pull/219)
Copy link
Contributor

Choose a reason for hiding this comment

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

reference #274 after #219

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@fanminshi
Copy link
Contributor

lgtm after nits

@fanminshi fanminshi merged commit 02cf852 into operator-framework:master May 23, 2018
m1kola pushed a commit to m1kola/operator-sdk that referenced this pull request Jun 7, 2024
…istency-openshift-4.12-openshift-enterprise-operator-sdk

Updating openshift-enterprise-operator-sdk images to be consistent with ART
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.

None yet

2 participants