Skip to content

Use Teleport's standard buildbox#17122

Merged
hugoShaka merged 4 commits into
masterfrom
hugo/17108-build-operator-in-global-buildbox
Oct 11, 2022
Merged

Use Teleport's standard buildbox#17122
hugoShaka merged 4 commits into
masterfrom
hugo/17108-build-operator-in-global-buildbox

Conversation

@hugoShaka
Copy link
Copy Markdown
Contributor

@hugoShaka hugoShaka commented Oct 6, 2022

Fixes #17108

The first commit edits the teleport-operator container image build process to rely on Teleport's standard buildbox. This will make sure we are using a single go version at all time.

It also removes unused environment variables from operator/Makefile.

The second commit extracts BUILDBOX_VERSION, BUILDBOX and BUILDBOX_variant variables into a standalone build.assets/images.mk file so they can be included by the other Makefiles in the repo and ensures all versions and repos are in-sync.

This comit edits the teleport-operator container image build process to
rely on Teleport's standard buildbox. This will make sure we are using a
single go version at all time.

This also removed unused environement variables from
`operator/Makefile`.
@hugoShaka hugoShaka requested a review from r0mant October 6, 2022 15:24
@github-actions github-actions Bot requested a review from xacrimon October 6, 2022 15:24
Copy link
Copy Markdown
Collaborator

@r0mant r0mant left a comment

Choose a reason for hiding this comment

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

@hugoShaka I would try to find a way to have the buildbox version defined in a single place (it's currently in build.assets/Makefile) but otherwise lgtm.

Also, can you please run a dev release off of this branch to make sure everything works?

Comment thread operator/Makefile Outdated
BUNDLE_GEN_FLAGS += --use-image-digests
endif
# Buildbox image version, can be built by running `make -C build.assets`
BUILDBOX_VERSION ?= teleport11
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is there any way this can somehow be propagated from build.assets/Makefile where it's already set? Ideally, version would be defined in a single place.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in 547976c , I also updated docker/Dockerfile

@r0mant r0mant requested a review from zmb3 October 6, 2022 16:05
@hugoShaka hugoShaka requested a review from r0mant October 6, 2022 16:31
Copy link
Copy Markdown
Contributor

@wadells wadells left a comment

Choose a reason for hiding this comment

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

Ship it! (pending a successful dev build).

I've got one running that includes these changes (as well as my own work that I was interested in testing):

https://drone.platform.teleport.sh/gravitational/teleport/16265

@github-actions github-actions Bot removed request for xacrimon and zmb3 October 8, 2022 06:06
@wadells
Copy link
Copy Markdown
Contributor

wadells commented Oct 8, 2022

The root cause of:

Step 18/19 : COPY --from=builder /go/src/github.com/gravitational/teleport/build/teleport-operator .
COPY failed: stat go/src/github.com/gravitational/teleport/build/teleport-operator: file does not exist

is https://forums.docker.com/t/resolved-files-missing-after-dockerfile-run-downloads-them/4827

The hack is to move the binary outside the /go/src/github.com/gravitational/teleport volume defined in the base image. /tmp maybe?

However, I wonder if we need the volume declared in the base image -- docker has no trouble mounting over paths even if they're not declared as volumes.

@hugoShaka
Copy link
Copy Markdown
Contributor Author

The hack is to move the binary outside the /go/src/github.com/gravitational/teleport volume defined in the base image. /tmp maybe?

What about putting the binaries in $GOPATH/bin ?

However, I wonder if we need the volume declared in the base image -- docker has no trouble mounting over paths even if they're not declared as volumes.

I think we don't strictly need those volumes, but they ensure we don't put anything in those paths during the image build process.

@wadells
Copy link
Copy Markdown
Contributor

wadells commented Oct 8, 2022

What about putting the binaries in $GOPATH/bin ?

Sounds good to me!

@hugoShaka hugoShaka enabled auto-merge (squash) October 11, 2022 14:58
@hugoShaka hugoShaka merged commit 2ef2de9 into master Oct 11, 2022
@github-actions
Copy link
Copy Markdown
Contributor

@hugoShaka See the table below for backport results.

Branch Result
branch/v10 Failed
branch/v11 Failed

hugoShaka added a commit that referenced this pull request Oct 11, 2022
* Use Teleport's standard buildbox

This commit edits the teleport-operator container image build process to
rely on Teleport's standard buildbox. This will make sure we are using a
single go version at all time.

This also removed unused environment variables from
`operator/Makefile`.

* Extract BUILDBOX variables out of build.assets/Makefile
* Put `teleport-operator` bin out of the Teleport source volume
hugoShaka added a commit that referenced this pull request Oct 11, 2022
* Use Teleport's standard buildbox

This commit edits the teleport-operator container image build process to
rely on Teleport's standard buildbox. This will make sure we are using a
single go version at all time.

This also removed unused environment variables from
`operator/Makefile`.

* Extract BUILDBOX variables out of build.assets/Makefile
* Put `teleport-operator` bin out of the Teleport source volume
@hugoShaka hugoShaka deleted the hugo/17108-build-operator-in-global-buildbox branch October 13, 2022 13:32
hugoShaka added a commit that referenced this pull request Oct 17, 2022
* Use Teleport's standard buildbox

This commit edits the teleport-operator container image build process to
rely on Teleport's standard buildbox. This will make sure we are using a
single go version at all time.

This also removed unused environment variables from
`operator/Makefile`.

* Extract BUILDBOX variables out of build.assets/Makefile
* Put `teleport-operator` bin out of the Teleport source volume
hugoShaka added a commit that referenced this pull request Oct 31, 2022
* Use Teleport's standard buildbox

This commit edits the teleport-operator container image build process to
rely on Teleport's standard buildbox. This will make sure we are using a
single go version at all time.

This also removed unused environment variables from
`operator/Makefile`.

* Extract BUILDBOX variables out of build.assets/Makefile
* Put `teleport-operator` bin out of the Teleport source volume
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Operator should use standard Teleport buildbox

4 participants