Skip to content
This repository has been archived by the owner on Nov 30, 2022. It is now read-only.

PLAT-1214: Upgrade to Go 1.16 #86

Merged
merged 4 commits into from
Jul 23, 2021
Merged

PLAT-1214: Upgrade to Go 1.16 #86

merged 4 commits into from
Jul 23, 2021

Conversation

fraenkel
Copy link
Contributor

Force rootless builds

There is currently no ideal why to detect when we are root but running
rootless.
For now just force rootless builds.

Support token authorization for docker registry v2

See https://docs.docker.com/registry/spec/auth/token/ for more detail

Update dependencies

Minor changes to get things compiling again.
Use embed instead of pkger

Simplify the makefile
Add a precommit target to verify no code generation is needed

runc 1.0.0
containerd 1.5.2
buildkit is using master to pull in latest rootless bits

tooling is now vendored as well
@fraenkel fraenkel requested review from a team, sonnysideup and steved June 30, 2021 16:35
Copy link
Contributor

@steved steved left a comment

Choose a reason for hiding this comment

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

Tested w/Domino too and LGTM. I'll let @sonnysideup have final say.

internal/builder/embedded/bkimage/push.go Outdated Show resolved Hide resolved
@fraenkel
Copy link
Contributor Author

fraenkel commented Jul 1, 2021

waiting on kustomize 4.2.0 binaries to appear...

Copy link
Contributor

@sonnysideup sonnysideup left a comment

Choose a reason for hiding this comment

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

@fraenkel Check out my comments and see what you think. We can have a zoom chat if that's easier.

Re: moving main.go to cmd/forge/main.go, I'm a little curious what your thoughts are here. I was originally thinking that I would produce 2 separate binaries in my upcoming changes:

  • controller
  • builder

This project really houses two separate applications. Separate apps/CLIs with reused libraries feels like a cleaner design as opposed to the current, dual-mode based on command weirdness we have now. Assuming we go that direction, removing main.go and setting the project up the way you've started makes sense to me. Was this the first step?

.circleci/config.yml Outdated Show resolved Hide resolved
.circleci/config.yml Outdated Show resolved Hide resolved
Dockerfile Show resolved Hide resolved
Dockerfile Show resolved Hide resolved
internal/crd/crd.go Outdated Show resolved Hide resolved
internal/crd/crd.go Outdated Show resolved Hide resolved
internal/builder/embedded/bkimage/workeropt.go Outdated Show resolved Hide resolved
internal/buildjob/buildjob.go Outdated Show resolved Hide resolved
@sonnysideup sonnysideup mentioned this pull request Jul 22, 2021
@fraenkel
Copy link
Contributor Author

@fraenkel Check out my comments and see what you think. We can have a zoom chat if that's easier.

Re: moving main.go to cmd/forge/main.go, I'm a little curious what your thoughts are here. I was originally thinking that I would produce 2 separate binaries in my upcoming changes:

  • controller
  • builder

This project really houses two separate applications. Separate apps/CLIs with reused libraries feels like a cleaner design as opposed to the current, dual-mode based on command weirdness we have now. Assuming we go that direction, removing main.go and setting the project up the way you've started makes sense to me. Was this the first step?

Would prefer separate binaries but it matters little given one image. Although they will be different sizes based on what is pulled in.

The actual impetus was to get embed to work at the root. It created 2 packages at the root so I just moved the one that made the most sense.

Michael Fraenkel added 3 commits July 23, 2021 10:56
Minor changes to get things compiling again.
Use embed instead of pkger

Simplify the makefile
Add a precommit target to verify no code generation is needed

client-gen expects API_DIR/group/version directories. If there is only
one level, it will treat it as a group with an internalversion, i.e.,
unspecified.
Upgrade to v4.2.0
Remove workarounds which are now unnecessary
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants