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

Change default path from /usr/local/kubebuilder/bin/ to something that doesn't require SUDO #1732

Closed
ellistarn opened this issue Oct 20, 2020 · 11 comments
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Milestone

Comments

@ellistarn
Copy link

It would be great if kubebuilder didn't require sudo to install (on most machines). /usr/local is typically a restricted area of the filesystem. Additionally, it doesn't automatically wire up commonly added PATH locations, so it must be individually added.

This may not be the best approach, but many tools use GOBIN which works very well for me.

I haven't found it

Nope

/kind feature
/kind documentation

@ellistarn ellistarn added the kind/feature Categorizes issue or PR as related to a new feature. label Oct 20, 2020
@k8s-ci-robot k8s-ci-robot added the kind/documentation Categorizes issue or PR as related to documentation. label Oct 20, 2020
@camilamacedo86
Copy link
Member

camilamacedo86 commented Oct 21, 2020

The default value is set up in the controller-runtime, however, the PR: #1711 will address that since the bins will be download per project.

@camilamacedo86 camilamacedo86 self-assigned this Oct 21, 2020
@ellistarn
Copy link
Author

I'm not sure what you mean by bins will be downloaded per project. Do you mean gobin?

@camilamacedo86
Copy link
Member

HI @ellistarn,

To change the default path we raised an issue in the controller-runtime to address it. See: kubernetes-sigs/controller-runtime#1234.

However, for v3+ plugin on the master, you can see that test target will download the bins in the project. Then, see that we ave been working on in a better solution in the PR: #1711

You can also, customize your project to work as was done in this PR until it in the introduced in the project.

@ellistarn
Copy link
Author

ellistarn commented Nov 3, 2020

Thank you! Just so I'm tracking this correctly. After PR #1711, the binaries will be provided in GOBIN?

@camilamacedo86
Copy link
Member

HI @ellistarn,

The #1711 proposes a solution where the env test binaries are downloaded in the bin/ directory when is the KUBEBUILDER_ASSETS is not set which means when the binaries are not found in the default path which is defined in controller-runtime.

In this way, IMO we need to change the default to be $GOBIN/ as you suggested instead of "/usr/local/kubebuilder/bin" which requires sudo permissions. And then, it is tracked in kubernetes-sigs/controller-runtime#1234. I will try to work on in the controller-runtime change as possible I have time. However, feel free if you wish to do this contribution as well.

So, after both be addressed we will end up with the following behaviour:

  • If the binaries are set up in the $GOBIN/ and the KUBEBUILDER_ASSETS be set the controllers will use it
  • Otherwise, the binaries will be added to the bin/ directory of the project.

Is it make sense?

@ellistarn
Copy link
Author

Yeah makes sense thank you. When does the download to /bin happen? Is it on testenv setup, or on project creation? I'm not super excited about checking the binaries in to /bin or adding /bin to my gitignore, so I definitely prefer the GOBIN solution. I'll take a look at what it might take to do this.

@camilamacedo86
Copy link
Member

camilamacedo86 commented Nov 3, 2020

Hi @ellistarn,

Users still not needing care about it at all. All complexities are abstracted by the tool.

When does the download to /bin happen? Is it on testenv setup, or on project creation?

It happens when the Makefile target test is called

I'm not super excited about checking the binaries into/bin or adding /bin to my gitignore

It is done by default already. The projects that are scaffolded by the tool has a .gitigore which is set up to ignore all that by default should not be commited.

@ellistarn
Copy link
Author

ellistarn commented Nov 3, 2020

Yeah I understand it's automatically scaffolded, but I think this still prevents a problem.

As a Kubebuilder user, one of the first things I did was remove a bunch of what I thought of as "kubebuilder cruft" from my project. These were things I thought were unnecessary or messy or confusing. For example I completely rewrote the makefile to rely on http://github.com/google/ko for a more streamlined build/deploy story. I also manage my dependencies (ginkgo, gomega, golanglint-ci) via a go module in tools/go.mod. I also completely rewrote the /config section as I disagreed with the organization.

The point of this isn't to say that kubebuilder should do all these things, but as an example of how and why users will diverge.
Even if it's scaffolded by the generator, details like /bin being on gitignore is something that users will need to be aware of (and why) and deal with indefinitely. For example, it will complicate projects where users might want to store other binaries in /bin.

I think Kubebuilder is an amazing starting point, but the team should be expecting their users to diverge pretty quickly to what makes the most sense for the user's development style and use case. In this sense, keeping opinions as optional as possible should be a goal.

@ellistarn
Copy link
Author

ellistarn commented Nov 3, 2020

In the case here, I'm quite excited by having kubebuilder vend its test binaries via GOBIN because that's how all my tools vend their binaries to my CI system

// +build tools

package tools

import (
	_ "github.com/fzipp/gocyclo"
	_ "github.com/golangci/golangci-lint/cmd/golangci-lint"
	_ "github.com/google/ko/cmd/ko"
	_ "github.com/onsi/ginkgo/ginkgo"
	_ "sigs.k8s.io/controller-tools/cmd/controller-gen"
	_ "sigs.k8s.io/kubebuilder/cmd"
)
cd tools; GO111MODULE=on cat tools.go | grep _ | awk -F'"' '{print $2}' | xargs -tI % go install %

This simple command installs my entire CI workflow onto a fresh VM.

Presently, I need to do a hack for kubebuilder (which requires sudo)

kubebuilder() {
    os=$(go env GOOS)
    arch=$(go env GOARCH)
    curl -L "https://go.kubebuilder.io/dl/2.3.1/${os}/${arch}" | tar -xz -C $TEMP_DIR
    sudo mkdir -p /usr/local/kubebuilder/bin/
    sudo mv $TEMP_DIR/kubebuilder_2.3.1_${os}_${arch}/bin/* /usr/local/kubebuilder/bin/
    echo 'Add kubebuilder to your path `export PATH=$PATH:/usr/local/kubebuilder/bin/`'
}

@camilamacedo86
Copy link
Member

Hi @ellistarn,

To solve your problem so far, note that you can set the env var KUBEBUILDER_ASSETS with the path that you would like to have the bins in your suite_test.go:

os.Setenv("KUBEBUILDER_ASSETS", filepath.Join(binPath, "bin")) 

See the example; https://github.com/operator-framework/operator-sdk/blob/6e5f58b07e26658bbcf34b9d467b91622b43e0c0/testdata/go/memcached-operator/controllers/suite_test.go

@camilamacedo86 camilamacedo86 modified the milestones: v3+ plugin, 3.1.0 Nov 17, 2020
@camilamacedo86
Copy link
Member

For the next release and go version plugin it will no longer be required use sudo permissions because the makefile target test will download the bins inside of the project. So, we can consider it as done.

However, it still not the ultimate solution desired and then, see that we have already tracked in controller-runtime an issue to change the default path for GOPATH/bin as suggested here. However, this change requires to get done there. More info: kubernetes-sigs/controller-runtime#1234

In this way, I am closing this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
3 participants