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

Make prep-local explicit and update docs #587

Merged
merged 2 commits into from Dec 12, 2017

Conversation

eriknelson
Copy link
Contributor

Describe what this PR does and why we need it:

I was continually running into unexpected behavior while resetting my catasb. Turns out the certs / tokens were getting regenerated in the cluster but were not being exported on my make runs because the files already existed.

I believe local developers should have an explicit "prep-local" target designed to setup a local environment with exported credentials, and a separate "make run" that is as thin as possible to facilitate rapid development iteration and testing.

@openshift-ci-robot openshift-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Dec 11, 2017
Makefile Outdated
# /var/run/secrets/kubernetes.io/serviceaccount
# Resetting a catasb cluster WILL generate new certs, so you will have to
# run prep-local again to export the new certs.
prep-local:
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this need to be in .PHONY list?

Copy link
Contributor

Choose a reason for hiding this comment

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

Add a comment after prep-local so that it shows up in the help.

prep-local: ## Prepares the local dev environment

Then test it with

$ make help
Usage: make <OPTIONS> ... <TARGETS>

Available targets are:

vendor               Install or update project dependencies
broker               Build the broker
build                Build binary from source
lint                 Run golint
fmt                  Run go fmt
fmtcheck             Check go formatting
test                 Run unit tests
...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a nifty trick.

CONTRIBUTING.md Outdated
@@ -164,9 +161,13 @@ cp scripts/my_local_dev_vars.example scripts/my_local_dev_vars
Now you can modify `scripts/my_local_dev_vars` with things like your `DOCKERHUB_USERNAME`
or use an insecure broker with `BROKER_INSECURE="true"`.

It is possible to use an etcd instance running locally on your host instead
of in-cluster. Simply set LOCAL_ETCD="true" in the my_local_dev_vars file,
Copy link
Contributor

Choose a reason for hiding this comment

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

change LOCAL_ETCD="true" to have the tick marks around it: LOCAL_ETCD="true"

CONTRIBUTING.md Outdated
@@ -207,6 +208,11 @@ registry:
org: example
```

**NOTE**: It is important to explicitly run `make prep-local` every time a new
cluster has been setup, like reseting a catasb cluster, for example. The reason
Copy link
Contributor

Choose a reason for hiding this comment

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

TYPO: reseting -> resetting

CONTRIBUTING.md Outdated
@@ -207,6 +208,11 @@ registry:
org: example
```

**NOTE**: It is important to explicitly run `make prep-local` every time a new
cluster has been setup, like reseting a catasb cluster, for example. The reason
is that the cluster's token/certs will have changed, so you will meed to `prep-local`
Copy link
Contributor

Choose a reason for hiding this comment

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

TYPO: meed to -> need to run

@eriknelson
Copy link
Contributor Author

@jmrodri Updated. You're getting good at copy edit with all that blogging :P

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling dad2bdd on eriknelson:explicit-prep-local into ** on openshift:master**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling dad2bdd on eriknelson:explicit-prep-local into ** on openshift:master**.

@jmrodri jmrodri requested a review from djzager December 11, 2017 19:38
Copy link
Member

@djzager djzager left a comment

Choose a reason for hiding this comment

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

LGTM.

@djzager djzager merged commit dccbe55 into openshift:master Dec 12, 2017
@eriknelson eriknelson deleted the explicit-prep-local branch December 12, 2017 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-review size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants