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

Add dockerfile for ppc64le and related changes #638

Merged
merged 3 commits into from
Aug 17, 2017

Conversation

hkshaw1990
Copy link
Contributor

A small change in the makefile determining the right architecture and a new Dockerfile.ppc64le where the base image is different (ppc64le/busybox:glibc).

Makefile Outdated
@@ -77,8 +78,14 @@ tarball: $(PROMU)
@$(PROMU) tarball --prefix $(PREFIX) $(BIN_DIR)

docker:
ifeq ($(MACH), ppc64le)
@echo ">> building docker image for ppc64le"
$(eval FILE_SUFFIX=.ppc64le)
Copy link
Member

Choose a reason for hiding this comment

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

I'd call it DOCKER_FILE and pass the full file. Could also happen setting as variable at the top.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, pushing a new commit.

Copy link
Member

@grobie grobie left a comment

Choose a reason for hiding this comment

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

Personally I'd keep all variable definitions at the top of the file and avoid eval. I guess it should be possible to set up to do the following:

  1. if DOCKERFILE is set already, don't do anything
  2. if DOCKERFILE isn't set use Dockerfile
  3. if DOCKERFILE isn't set and arch is ppc64le, use Dockerfile.ppc64le.

@matthiasr surely knows what I want ;-) It's not important though, we can change that later.

Makefile Outdated
@@ -77,8 +79,11 @@ tarball: $(PROMU)
@$(PROMU) tarball --prefix $(PREFIX) $(BIN_DIR)

docker:
ifeq ($(MACH), ppc64le)
$(eval DOCKERFILE=Dockerfile.ppc64le)
endif
@echo ">> building docker image"
Copy link
Member

Choose a reason for hiding this comment

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

Let's include the dockerfile we're using in this message. Something like >> building docker image from $(DOCKERFILE).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Included the dockerfile in the build message.

Copy link
Member

@grobie grobie left a comment

Choose a reason for hiding this comment

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

Thanks!

@grobie grobie merged commit de08e38 into prometheus:master Aug 17, 2017
oblitorum pushed a commit to shatteredsilicon/node_exporter that referenced this pull request Apr 9, 2024
* Add dockerfile for ppc64le and related changes

* Pass the fill file as DOCKEFILE

* Add the dockerfile name to build msg
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.

2 participants