Skip to:

  1. Skip to Jira Navigation
  2. Skip to Side Navigation
  3. Skip to Main Content

remove image related defaults from Docker container makefile

Description

The opennms-container/minion/Makefile defines a couple of docker image related defaults:

DOCKER_REGISTRY := docker.io DOCKER_ORG := opennms DOCKER_PROJECT := minion DOCKER_TAG := $(DOCKER_REGISTRY)/$(DOCKER_ORG)/$(DOCKER_PROJECT):$(VERSION) DOCKER_ARCH := linux/amd64

CircleCI's config.yml also defines image related defaults:

echo "export MINION_IMAGE=docker.io/opennms/minion" >> $BASH_ENV echo "export MINION_IMAGE_VERSION=\"$MINION_IMAGE_VERSION\"" >> $BASH_ENV

Maintainance of different build variants (horizon vs. meridian) is eased, if the Makefile does not defines its own defaults but is called with all arguments specified by the pipeline. Docker image related differences between builds would be concentrated at one place only.

Acceptance / Success Criteria

None

Lucidchart Diagrams

Activity

Show:

Morteza January 18, 2023 at 7:59 PM

I have pushed my changes to foundation-2023

Morteza January 18, 2023 at 3:55 PM

Morteza January 16, 2023 at 4:18 PM

I have created https://github.com/OpenNMS/opennms/pull/5690. With this change, my aim is to reduce the duplicate entries (where possible).

I’ll deliver my change to develop branch initially and back port it as required.

DJ Gregor January 12, 2023 at 6:07 PM
Edited

I definitely agree with trying to keep things DRY, however opennms-container/*/Makefile aren’t just used in CI, they’re also used by developers, so we’ll need to have reasonable defaults for developer use in the Makefiles. A lot of deduplication has already happened with the introduction of opennms-container/common.mk and it looks like there’s been some work since this ticket was opened in 2021 to get down to using common variable names (e.g. MINION_IMAGE_VERSION seems to no longer be around).

I’m not sure how much is left to do….

It feels like there is some opportunity to deduplicate the image tag and/or OCI file name. It looks like we have “DOCKER_CLI_EXPERIMENTAL: enabled” set in a number of places.

I think we could stand to deduplicate the “deploy-base” image and move it into common.mk and remove it from the Dockerfiles (it’s also referenced in a smoke test, I just noticed).

Random thing I noticed: this is defined but never used?

REMMDGREGOR (⎈|minikube:default) opennms dgregor$ git grep DOCKER_IMAGE_VERSION .circleci/main/aliases/general.yml: echo "export DOCKER_IMAGE_VERSION=\"$DOCKER_IMAGE_VERSION\"" >> $BASH_ENV

Related to this ticket, but out of scope for variable: It looks like we do ‘docker login’, ‘docker tag’, and ‘docker push’ in a number of places. Maybe some of these could be centralized (not sure if bash is the right place or makefiles).

Jeff Gehlbach January 11, 2023 at 2:40 PM

This work feels on the critical path for hosting Meridian container images on a non-DockerHub registry.

Fixed

Details

Assignee

Reporter

Sprint

Fix versions

Priority

Parent

PagerDuty

Created September 21, 2021 at 11:42 AM
Updated January 18, 2023 at 9:30 PM
Resolved January 18, 2023 at 9:30 PM

Flag notifications