← Back to team overview

bind-charmers team mailing list archive

Re: [Merge] ~barryprice/charm-k8s-bind/+git/bind-k8s-image-builder:master into ~bind-charmers/charm-k8s-bind/+git/bind-k8s-image-builder:master

 

Review: Needs Fixing

Some comments inline

Diff comments:

> diff --git a/.gitignore b/.gitignore
> new file mode 100644
> index 0000000..baa4b73
> --- /dev/null
> +++ b/.gitignore
> @@ -0,0 +1,8 @@
> +*.pyc
> +*.swp
> +*~
> +__pycache__
> +files/plugins/

Is this really needed?

> +files/themes/

Is this really needed?

> +.coverage
> +.tox/
> diff --git a/Dockerfile b/Dockerfile
> new file mode 100644
> index 0000000..32d8813
> --- /dev/null
> +++ b/Dockerfile
> @@ -0,0 +1,30 @@
> +ARG DIST_RELEASE
> +
> +FROM ubuntu:${DIST_RELEASE}
> +
> +LABEL maintainer="bind-charmers@xxxxxxxxxxxxxxxxxxx"
> +
> +ARG BUILD_DATE
> +ARG PKGS_TO_INSTALL
> +
> +LABEL org.label-schema.build-date=${BUILD_DATE}
> +
> +ENV BIND_CONFDIR=/etc/bind
> +
> +# Avoid interactive prompts
> +RUN echo 'debconf debconf/frontend select Noninteractive' | debconf-set-selections
> +
> +# Update all packages, remove cruft, install required packages, configure apache

configure apache?

> +RUN apt-get update && apt-get -y dist-upgrade \
> +    && apt-get --purge autoremove -y \
> +    && apt-get install -y ${PKGS_TO_INSTALL}
> +
> +# entrypoint script will configure Bind based on env variables
> +COPY ./files/docker-entrypoint.sh /usr/local/bin/
> +RUN chmod 0755 /usr/local/bin/docker-entrypoint.sh
> +
> +EXPOSE 53/udp
> +EXPOSE 53/tcp
> +
> +ENTRYPOINT ["/usr/local/bin/docker-entrypoint.sh"]
> +CMD /usr/sbin/named -f -u bind
> diff --git a/Makefile b/Makefile
> new file mode 100644
> index 0000000..95b1519
> --- /dev/null
> +++ b/Makefile
> @@ -0,0 +1,23 @@
> +DIST_RELEASE ?= focal
> +DOCKER_DEPS = bind9
> +
> +build-image:
> +	@echo "Building the image."
> +	@docker build \
> +		--no-cache=true \
> +		--build-arg BUILD_DATE=$$(date -u +'%Y-%m-%dT%H:%M:%SZ') \
> +		--build-arg PKGS_TO_INSTALL='$(DOCKER_DEPS)' \
> +		--build-arg DIST_RELEASE=$(DIST_RELEASE) \
> +		-t bind:$(DIST_RELEASE)-latest \
> +		.
> +
> +build: build-image

Let's just remove this since we don't want references to a non-public registry in a public branch

> +	@echo "Pushing to the prod-is-external registry."
> +	@docker tag bind:$(DIST_RELEASE)-latest prod-is-external.docker-registry.canonical.com/bind:$(DIST_RELEASE)-latest
> +	@docker push prod-is-external.docker-registry.canonical.com/bind:$(DIST_RELEASE)-latest
> +
> +clean:
> +	@echo "Cleaning files"
> +	@git clean -fXd
> +
> +.PHONY: build-image build clean 


-- 
https://code.launchpad.net/~barryprice/charm-k8s-bind/+git/bind-k8s-image-builder/+merge/387800
Your team Bind Charmers is requested to review the proposed merge of ~barryprice/charm-k8s-bind/+git/bind-k8s-image-builder:master into ~bind-charmers/charm-k8s-bind/+git/bind-k8s-image-builder:master.


References