-
Notifications
You must be signed in to change notification settings - Fork 237
Simplify and improve the local developer experience for pushing images #167
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Big improvement but still more to cut 🙂
SRC_DIRS := cmd pkg test/examples | ||
REGISTRY_STRING := $(subst /,_,$(REGISTRY)) | ||
CMD_DIRECTORIES := $(sort $(dir $(wildcard ./cmd/*/))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No longer used
SRC_DIRS := cmd pkg test/examples | ||
REGISTRY_STRING := $(subst /,_,$(REGISTRY)) | ||
CMD_DIRECTORIES := $(sort $(dir $(wildcard ./cmd/*/))) | ||
COMMANDS := $(CMD_DIRECTORIES:./cmd/%/=%) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No longer used
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still used / referenced. Will remove in a subsequent PR when fixing the build step to keep things clean.
Makefile
Outdated
CMD_DIRECTORIES := $(sort $(dir $(wildcard ./cmd/*/))) | ||
COMMANDS := $(CMD_DIRECTORIES:./cmd/%/=%) | ||
CONTAINER_FILES := $(addprefix .container-$(REGISTRY_STRING)-,$(addsuffix -$(VERSION),$(COMMANDS))) | ||
PUSH_FILES := $(addprefix .push-$(REGISTRY_STRING)-,$(addsuffix -$(VERSION),$(COMMANDS))) | ||
|
||
ARCH := amd64 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be ?=
Makefile
Outdated
CMD_DIRECTORIES := $(sort $(dir $(wildcard ./cmd/*/))) | ||
COMMANDS := $(CMD_DIRECTORIES:./cmd/%/=%) | ||
CONTAINER_FILES := $(addprefix .container-$(REGISTRY_STRING)-,$(addsuffix -$(VERSION),$(COMMANDS))) | ||
PUSH_FILES := $(addprefix .push-$(REGISTRY_STRING)-,$(addsuffix -$(VERSION),$(COMMANDS))) | ||
|
||
ARCH := amd64 | ||
OS := linux |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be ?=
Makefile
Outdated
TENANT := "spinnaker" | ||
endif | ||
|
||
ROOT_DIR := $(shell dirname $(abspath $(lastword $(MAKEFILE_LIST)))) | ||
PKG := github.com/oracle/mysql-operator | ||
REGISTRY := iad.ocir.io/$(TENANT) | ||
REGISTRY := iad.ocir.io |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be ?=
Makefile
Outdated
@@ -18,19 +18,16 @@ ifdef WERCKER | |||
TENANT := "oracle" | |||
else | |||
NEW_NAMESPACE ?= e2e-${USER} | |||
VERSION := ${USER}-$(shell date +%Y%m%d%H%M%S) | |||
VERSION := ${USER}-$(shell git describe --always --dirty) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be ?=
Makefile
Outdated
@@ -18,19 +18,16 @@ ifdef WERCKER | |||
TENANT := "oracle" | |||
else | |||
NEW_NAMESPACE ?= e2e-${USER} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No longer used
Makefile
Outdated
@@ -18,19 +18,16 @@ ifdef WERCKER | |||
TENANT := "oracle" | |||
else | |||
NEW_NAMESPACE ?= e2e-${USER} | |||
VERSION := ${USER}-$(shell date +%Y%m%d%H%M%S) | |||
VERSION := ${USER}-$(shell git describe --always --dirty) | |||
TENANT := "spinnaker" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be ?=
|
||
.PHONY: bin-clean | ||
bin-clean: | ||
clean: | ||
rm -rf .go bin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to rm .go
as it's not created any more AFAIK
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still created on my machine. Will remove after fixing the build process in a subsequent PR.
Makefile
Outdated
@docker push $(REGISTRY)/$*:$(VERSION) | ||
@docker images -q $(REGISTRY)/$*:$(VERSION) > $@ | ||
push: build | ||
@docker build --build-arg=http_proxy --build-arg=https_proxy -t $(REGISTRY)/$(TENANT)/mysql-operator:$(VERSION) -f docker/mysql-operator/Dockerfile . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would break these up over multiple lines
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree. Split into a separate step for hygiene and broken into multiple lines.
This change makes some simplifications to the Makefile around pushing development images.