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

NG.mk: A Next Generation Makefile #3505

Closed
wants to merge 1 commit into from
Closed

Conversation

alrs
Copy link
Contributor

@alrs alrs commented Oct 1, 2017

This adds NG.mk and some -ng Dockerfiles in the images/ directory.

This ng Makefile has only been tested with the ci and version-dist targets. Most other targets have been removed to be added back individually.

This Makefile builds all of the distributable binaries in Docker containers that mount the source tree read-only. This protects the source tree from root-owned artifacts and strange side-effects from inside containers.

This Makefile uses templates to build most of the binaries, which DRYs out the build targets substantially.

This Makefile does not use .PHONY for non-phony targets.

This Makefile uses pigz instead of gzip if available, which uses all cores and is considerably faster.

make -f NG.mk -j 3 ci version-dist is concurrency-safe and allows tests to run while kubectl and required Docker containers are downloaded in the background.

This Makefile has been tested to work with GNU Make 3.8 and 4.1.

Where possible, Docker containers have been changed to use the debian:stretch-slim image, as it is the same size as the existing image but a newer release of Debian. This isn't done for the utils-builder-ng image, as socat doesn't build using the provided incantation of gcc flags on Debian stable.

During development of this branch, I've turned on full-VM support in Travis-CI and tested that make -f NG.mk version-dist will spin up build containers and create the expected artifacts. It does.

This Makefile tweaks the layout of the .build directory. This will most likely not be an issue as new upload- targets are written to the new layout, but I'm all ears if the old way of having multiple copies of each binary was necessary for some reason.

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Oct 1, 2017
@k8s-github-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
We suggest the following additional approver: chrislovecnm

Assign the PR to them by writing /assign @chrislovecnm in a comment when ready.

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

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@alrs
Copy link
Contributor Author

alrs commented Oct 1, 2017

NG.mk gets interesting on line 530. It only works if the .dockerignore in the root of the project is deleted.

@alrs
Copy link
Contributor Author

alrs commented Oct 1, 2017

make -f NG.mk linux is a good example of this experiment at work.

@k8s-github-robot
Copy link

@alrs PR needs rebase

@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 4, 2017
@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 4, 2017
@alrs
Copy link
Contributor Author

alrs commented Oct 7, 2017

I'm able to replicate my problems with Travis in a local Ubuntu 14 container, so I think my problems in Travis are down to the differences between make 3.8 and 4.1.

@chrislovecnm
Copy link
Contributor

@alrs I had some problems with make 4.0 in the go container today. Take a look at #3556

@alrs alrs changed the title WIP: makefile-ng NG.mk: A Next Generation Makefile Oct 11, 2017
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 11, 2017
@chrislovecnm
Copy link
Contributor

  1. Did you port the updates from the main makefile file to here?
  2. Are we able to test this on a mac?
  3. We need the bazel targets as well.
  4. What version of make is required?
  5. We probably want to merge and test some.

One thing we noticed is that mac docker does not like checking timestamps on a mac hard drive. Ran into that with the protokube tar ball.

Copy link
Contributor

@chrislovecnm chrislovecnm left a comment

Choose a reason for hiding this comment

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

First review. Probably need a couple more

# See the License for the specific language governing permissions and
# limitations under the License.

FROM debian:stretch-slim
Copy link
Contributor

Choose a reason for hiding this comment

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

We do not use stretch slim. We use the official Debian k8s container.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm happy to put it back, as I mentioned in the PR description it's Debian stable instead of oldstable.

FROM debian:stretch-slim
ARG KOPS_SERVER
# TODO
# RUN apt-get update && apt-get install --yes --reinstall lsb-base
Copy link
Contributor

Choose a reason for hiding this comment

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

Todo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy-pasted from the original kops-server image Dockerfile in images/kops-server/Dockerfile. I can't speak to what needs to be done.


cd /socat && \
CFLAGS=-static LDFLAGS=-static CPPFLAGS=-static CFLAGS_APPEND=-static \
LDFLAGS_APPEND=-static CPPFLAGS_APPEND=-static \
Copy link
Contributor

Choose a reason for hiding this comment

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

Wondering if we can just have bazel do this now ... not certain

@alrs
Copy link
Contributor Author

alrs commented Oct 11, 2017 via email

@chrislovecnm
Copy link
Contributor

Not using .phony with a mac is not working with the protokube tarball :( I a running an older version of OSX, not sure what @justinsb is running. He works off of both Linux and mac.

@alrs
Copy link
Contributor Author

alrs commented Oct 11, 2017

@chrislovecnm Could you create a gist of the output of make -f NG.mk clean && make -f NG.mk ci version-dist?

@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Oct 13, 2017
@chrislovecnm
Copy link
Contributor

make -f NG.mk test version-dist |& tee /tmp/debug.out

First error

for t in upup/models/bindata.go federation/model/bindata.go; do if test -e $t; then rm -fv $t; fi; done
upup/models/bindata.go
federation/model/bindata.go
if test -e .build; then rm -rfv .build; fi
.build/local/go-bindata
.build/local
.build
mkdir -p .build/kops/1.8.0-alpha.1/linux/amd64
docker run -w /go/src/k8s.io/kops \
	-e "GOOS=linux" -e "GOARCH=amd64" -e "STATIC_BUILD=yes" \
	--name kops-build-linux-amd64-kops-1508544772 \
	-v `pwd`:/go/src/k8s.io/kops:ro \
	golang:1.8.3 \
	go build  \
	-ldflags "-X k8s.io/kops.Version=1.8.0-alpha.1 \
	-X k8s.io/kops.GitVersion=b334d963d " \
	-o /tmp/.build/kops/1.8.0-alpha.1/linux/amd64/kops ./cmd/kops/
federation/apply_federation.go:33:2: no buildable Go source files in /go/src/k8s.io/kops/federation/model
make: *** [.build/kops/1.8.0-alpha.1/linux/amd64/kops] Error 1

@chrislovecnm
Copy link
Contributor

This fails:

$ S3_BUCKET=s3://clove-kops make -f NG.mk kops upload
make: *** No rule to make target `kops'.  Stop.

@chrislovecnm
Copy link
Contributor

Latest error:

$ gmake -f NG.mk test version-dist |& tee /tmp/debug.out

redacted lots of content ... then

for t in upup/models/bindata.go federation/model/bindata.go; do if test -e $t; then rm -fv $t; fi; done
upup/models/bindata.go
federation/model/bindata.go
if test -e .build; then rm -rfv .build; fi
.build/artifacts/channels
.build/artifacts/protokube
.build/artifacts
.build/dist/images
.build/dist
.build/local/channels
.build/local/go-bindata
.build/local/kops
.build/local/protokube
.build/local
.build
mkdir -p .build/kops/1.8.0-alpha.1/linux/amd64
docker run -w /go/src/k8s.io/kops -e "GOOS=linux" -e "GOARCH=amd64" -e "STATIC_BUILD=yes" --name kops-build-kops-linux-amd64-1510276183 -v `pwd`:/go/src/k8s.io/kops:ro golang:1.8.3 go build  -ldflags "-X k8s.io/kops.Version=1.8.0-alpha.1 -X k8s.io/kops.GitVersion=35a338fda " -o /tmp/.build/kops/1.8.0-alpha.1/linux/amd64/kops ./cmd/kops/
federation/apply_federation.go:33:2: no buildable Go source files in /go/src/k8s.io/kops/federation/model
gmake: *** [NG.mk:288: .build/kops/1.8.0-alpha.1/linux/amd64/kops] Error 1
$ gmake --version
GNU Make 4.2.1
Built for x86_64-apple-darwin15.6.0
Copyright (C) 1988-2016 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
$ docker version
Client:
 Version:      17.09.0-ce
 API version:  1.32
 Go version:   go1.8.3
 Git commit:   afdb6d4
 Built:        Tue Sep 26 22:40:09 2017
 OS/Arch:      darwin/amd64

Server:
 Version:      17.09.0-ce
 API version:  1.32 (minimum version 1.12)
 Go version:   go1.8.3
 Git commit:   afdb6d4
 Built:        Tue Sep 26 22:45:38 2017
 OS/Arch:      linux/amd64
 Experimental: true
$ uname -a
Darwin clove-mbp.local 15.6.0 Darwin Kernel Version 15.6.0: Mon Jan  9 23:07:29 PST 2017; root:xnu-3248.60.11.2.1~1/RELEASE_X86_64 x86_64

Yah I need to upgrade

@chrislovecnm
Copy link
Contributor

mkdir -p .build/kops/1.8.0-alpha.1/linux/amd64
docker run -w /go/src/k8s.io/kops -e "GOOS=linux" -e "GOARCH=amd64" -e "STATIC_BUILD=yes" --name kops-build-kops-linux-amd64-1510277483 -v `pwd`:/go/src/k8s.io/kops:ro golang:1.8.3 go build  -ldflags "-X k8s.io/kops.Version=1.8.0-alpha.1 -X k8s.io/kops.GitVersion=35a338fda " -o /tmp/.build/kops/1.8.0-alpha.1/linux/amd64/kops ./cmd/kops/
federation/apply_federation.go:33:2: no buildable Go source files in /go/src/k8s.io/kops/federation/model
make: *** [.build/kops/1.8.0-alpha.1/linux/amd64/kops] Error 1

Same error with regular make

@chrislovecnm
Copy link
Contributor

$ docker ps
CONTAINER ID        IMAGE               COMMAND                  CREATED             STATUS              PORTS               NAMES
ccb351a525e1        golang:1.8.3        "make -C /go/src/k..."   About an hour ago   Up About an hour                        nodeup-build-1510284565
8997b831818a        golang:1.8.3        "make -C /go/src/k..."   About an hour ago   Up About an hour                        kops-build-1510284565

I having hanging jobs ...

@justinsb justinsb added this to the 1.9 milestone Nov 22, 2017
@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 20, 2018
@justinsb justinsb modified the milestones: 1.9.0, 1.9.1 Feb 21, 2018
@fejta-bot
Copy link

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten
/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Mar 23, 2018
@fejta-bot
Copy link

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants