refactor: replace go-bindata with native embed package

[muicoder]

Proposed Changes

• Based on the PR of Issues refactor: Use embed package for manifests directory #12670

Types of Changes

tech debt

Verification

no more codegen steps in CI, no more go-bindata in go.mod

Testing

yes

Linked Issues

• Replace go-bindata with embed #12683

User-Facing Change

Further Comments

Thanks to @eugercek for pointing out that this was something that needed attention, and an initial PR for partial replacement of go-bindata.

[brandond]

LGTM, except for one thing as discussed at #12670 (comment) - please add build tags to deploy/data.go and static/data.go:

//go:build !no_stage
// +build !no_stage

[brandond]

Hmm, it looks like this significantly increases the size of the k3s binary:

k3s binary dist/artifacts/k3s size 113377464 exceeds max acceptable size of 78643200 bytes (75 MiB)

go-bindata did have built-in compression support, which go:embed does not. We'll need to address that somehow.

[muicoder]

This occurs when referencing github.com/k3s-io/k3s/pkg/util/embedutil.go in pkg/data/data.go,so the AssetNames and Asset implementations of these 2 packages are separated.

+ cp -av manifests pkg/deploy/embed
'manifests' -> 'pkg/deploy/embed'
'manifests/ccm.yaml' -> 'pkg/deploy/embed/ccm.yaml'
'manifests/coredns.yaml' -> 'pkg/deploy/embed/coredns.yaml'
'manifests/local-storage.yaml' -> 'pkg/deploy/embed/local-storage.yaml'
'manifests/metrics-server' -> 'pkg/deploy/embed/metrics-server'
'manifests/metrics-server/aggregated-metrics-reader.yaml' -> 'pkg/deploy/embed/metrics-server/aggregated-metrics-reader.yaml'
'manifests/metrics-server/auth-delegator.yaml' -> 'pkg/deploy/embed/metrics-server/auth-delegator.yaml'
'manifests/metrics-server/auth-reader.yaml' -> 'pkg/deploy/embed/metrics-server/auth-reader.yaml'
'manifests/metrics-server/metrics-apiservice.yaml' -> 'pkg/deploy/embed/metrics-server/metrics-apiservice.yaml'
'manifests/metrics-server/metrics-server-deployment.yaml' -> 'pkg/deploy/embed/metrics-server/metrics-server-deployment.yaml'
'manifests/metrics-server/metrics-server-service.yaml' -> 'pkg/deploy/embed/metrics-server/metrics-server-service.yaml'
+ cp -av build/static pkg/static/embed
'build/static' -> 'pkg/static/embed'
'build/static/charts' -> 'pkg/static/embed/charts'
'build/static/charts/traefik-crd-34.2.1+up34.2.0.tgz' -> 'pkg/static/embed/charts/traefik-crd-34.2.1+up34.2.0.tgz'
'build/static/charts/traefik-34.2.1+up34.2.0.tgz' -> 'pkg/static/embed/charts/traefik-34.2.1+up34.2.0.tgz'
+ cp -av build/data pkg/data/embed
'build/data' -> 'pkg/data/embed'
'build/data/79331e8a48ca31e01b9ef36e15be4c987ee87dca459ecbb6ee232856ec375bdf.tar.zst' -> 'pkg/data/embed/79331e8a48ca31e01b9ef36e15be4c987ee87dca459ecbb6ee232856ec375bdf.tar.zst'

Above are the specific files for the 3 embeds, which have already been compressed except for the yaml text.
Secondary compression of already compressed files will not result in a significant reduction in size.

[brandond]

Ah right. I think we can avoid duplication while also handling it with a common wrapper if we just put it in a separate package.

I've pushed another commit to your PR branch with a couple suggested enhancements, hope you don't mind.

[muicoder]

LGTM

[brandond]

I don't like the variable and rm; cp pattern. Why is that necessary? I see that you're doing this to avoid embedding the the .empty file but according to the embed docs:

If a pattern names a directory, all files in the subtree rooted at that directory are embedded (recursively), except that files with names beginning with ‘.’ or ‘_’ are excluded.

Without the .empty file, go vet fails:

brandond@dev01:~/go/src/github.com/k3s-io/k3s$ go vet ./pkg/...
pkg/data/data.go:9:12: pattern embed/*: no matching files found

So I think the correct fix would be to revert your last change, and simply change the embeds to refer to the directory, instead of globbing within that directory:

//go:embed embed
var embedFS embed.FS

[brandond]

Indeed. I guess it's not really designed to be used like this, where the embedded files are not always present.

Maybe we leave the glob, but modify the AssetNames function to skip files starting with . or _, with a comment explaining why?

[muicoder]

Indeed. I guess it's not really designed to be used like this, where the embedded files are not always present.

Maybe we leave the glob, but modify the AssetNames function to skip files starting with . or _, with a comment explaining why?

Nice way to skip in AssetNames, committed.

[manuelbuil]

Won't we need to change the BUILD instructions?

[brandond]

@manuelbuil how so? We only support building via build scripts. Just running go build in the project has never been sufficient.

[manuelbuil]

@manuelbuil how so? We only support building via build scripts. Just running go build in the project has never been sufficient.

I we do away with all codegen entirely we should change this line https://github.com/k3s-io/k3s/blob/master/BUILDING.md?plain=1#L14, right? And thereby this file https://github.com/k3s-io/k3s/blob/master/scripts/generate

[brandond]

@manuelbuil oh I missed that one because it is "${GO}" generate for some reason instead of just go generate and it didn't turn up in my search. I'll update.

[brandond]

Thanks @muicoder @eugercek!