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

Fixes #2971, adding a section to the tutorial for using minikube when running locally #3010

Merged
merged 1 commit into from
Jul 30, 2020

Conversation

psschwei
Copy link
Contributor

@psschwei psschwei commented Jul 24, 2020

Changes

This PR fixes #2971, adding a section to the tutorial for using minikube when running locally.

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

Reviewer Notes

If API changes are included, additive changes must be approved by at least two OWNERS and backwards incompatible changes must be approved by more than 50% of the OWNERS, and they must first be added in a backwards compatible way.

/kind documentation
/release-note-none

@tekton-robot tekton-robot added kind/documentation Categorizes issue or PR as related to documentation. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Jul 24, 2020
@tekton-robot tekton-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jul 24, 2020
@psschwei
Copy link
Contributor Author

/release-note-none

@tekton-robot tekton-robot added release-note-none Denotes a PR that doesnt merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Jul 24, 2020
@psschwei
Copy link
Contributor Author

To whomever reviews this -- I used my best judgement for updating the format of the tutorial (by adding sections for Docker for Desktop and Minikube). But can revise if others have a more preferred layout.

@psschwei
Copy link
Contributor Author

/kind documentation

@psschwei
Copy link
Contributor Author

/remove-kind documentation

@tekton-robot tekton-robot removed the kind/documentation Categorizes issue or PR as related to documentation. label Jul 27, 2020
@psschwei
Copy link
Contributor Author

/kind documentation

@tekton-robot tekton-robot added the kind/documentation Categorizes issue or PR as related to documentation. label Jul 27, 2020

You must reconfigure any `image` resource definitions in your `PipelineResources` as follows:

- Set the URL to `example.com/<image_name>`
Copy link
Member

Choose a reason for hiding this comment

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

@psschwei I am not able to understand instructions in this section, what is this URL meant for? we enabled registry in the previous section. Also, since we are using -L flag, no need to setup KO related env (i.e. unlike KO_DOCKER_REPO for docker for desktop). What would this example.com be in a minikube installation?

Copy link
Contributor Author

@psschwei psschwei Jul 29, 2020

Choose a reason for hiding this comment

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

example.com is one of the aliases created automatically for the registry via the registry-aliases addon (others are example.org, test.com, and test.org):

docker@minikube:~$ cat /etc/hosts
127.0.0.1	localhost
::1	localhost ip6-localhost ip6-loopback
fe00::0	ip6-localnet
ff00::0	ip6-mcastprefix
ff02::1	ip6-allnodes
ff02::2	ip6-allrouters
172.17.0.3	minikube
172.17.0.1	host.minikube.internal
172.17.0.3	control-plane.minikube.internal
10.104.170.3	example.org
10.104.170.3	example.com
10.104.170.3	test.com
10.104.170.3	test.org

(10.104.170.3 is the ClusterIP of the registry service)

If we want to be able to push/pull to the minikube Docker registry, we'll need to use one of its aliases. If we don't, it will try to resolve to Docker Hub -- this is the error I got when running the build-docker-image-from-git-source-task-run TaskRun using just leeroy-web as the URL for the skaffold-image-leeroy-web PipelineResource:

error checking push permissions -- make sure you entered the correct tag name, and that 
you are authenticated correctly, and try again: checking push permission for "leeroy-web": 
POST https://index.docker.io/v2/library/leeroy-web/blobs/uploads/: UNAUTHORIZED: 
authentication required; [map[Action:pull Class: Name:library/leeroy-web Type:repository] 
map[Action:push Class: Name:library/leeroy-web Type:repository]]

@pritidesai I can add some clarifying remarks to the Reconfigure image resources section to explain why we're using example.com as the registry alias here if that would be useful. While we could use an alternative hostname (for example, host.docker.local like in the Docker for Desktop example), it would require editing /etc/hosts and the CoreDNS configmap, so I assumed the simpler option was better. :)

Copy link
Member

Choose a reason for hiding this comment

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

Thanks @psschwei for the explanation, you can tell how much I know minikube configuration 🤦‍♀️

This looks good, no need to add alternative hostname. It will be great to clarify why example.com and adding may be this command to get the list of available hostnames minikube ssh -- cat /etc/hosts

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No worries -- I only learned this in the course of doing this PR (and from a very helpful suggestion from @kameshsampath ). Will make the update shortly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Update made

@psschwei psschwei force-pushed the tutorial-minikube branch from c675461 to d6d09e8 Compare July 30, 2020 01:25
@pritidesai
Copy link
Member

thanks @psschwei

/approve

@tekton-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pritidesai

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@tekton-robot tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 30, 2020

### Prerequisites
* [Docker for Desktop](#prerequisites-docker-for-desktop)
Copy link

Choose a reason for hiding this comment

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

minor nit, suggest matching bullet style with rest of document (using - instead of *)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sbwsg was going to fix this, but looks like the PR already merged... I can open another one that'll clean this up (and fix the resource usage too), or wait and include it alongside something more meaningful if you think this is too trivial for a standalone PR.

Copy link

Choose a reason for hiding this comment

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

Small PRs are good PRs :). Fixing in a follow-up is totally a good way to go. Thanks @psschwei !

- Install the [required tools](https://github.com/tektoncd/pipeline/blob/master/DEVELOPMENT.md#requirements).
- Install [minikube](https://kubernetes.io/docs/tasks/tools/install-minikube/) and start a sesion as follows:
```bash
minikube start --memory 10240 --cpus 6
Copy link

Choose a reason for hiding this comment

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

fwiw I develop Tekton in a minikube with 6GB RAM and 2 cpus and things run really nicely. (this is using the virtualbox driver on mac)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Happy to lower this -- I just mirrored the specs that were listed from the Docker for Desktop section : "Install Docker for Desktop and configure it to use six CPUs, 10 GB of RAM and 2GB of swap space."

@ghost
Copy link

ghost commented Jul 30, 2020

/lgtm

@tekton-robot tekton-robot assigned ghost Jul 30, 2020
@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 30, 2020
@tekton-robot tekton-robot merged commit e175400 into tektoncd:master Jul 30, 2020
@psschwei psschwei deleted the tutorial-minikube branch July 30, 2020 19:08
@psschwei psschwei restored the tutorial-minikube branch July 30, 2020 19:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/documentation Categorizes issue or PR as related to documentation. lgtm Indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesnt merit a release note. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tutorial: enhance "running this tutorial locally"
3 participants