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.

Sign up for GitHub

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

Jump to bottom

Fix and optimize generator Docker image building #1045

Merged
merged 2 commits into from
Nov 21, 2023
Merged

Fix and optimize generator Docker image building #1045

merged 2 commits into from
Nov 21, 2023

Conversation

hhromic
Copy link
Contributor

@hhromic hhromic commented Nov 11, 2023 •
edited
Loading

This PR implements a fix and an optimization to the generator Docker image building as described below.
To facilitate review, these two improvements were implemented in two separate commits.

  • Fix Docker image of generator always pointing to latest tag
    • new REPO_TAG variable can be used to specify the tag/branch to use for a build
    • the Makefile default is to use the branch of the current repository HEAD
    • the Dockerfile default (if building manually with Docker) is main
    • updated CircleCI configuration to apply the correct REPO_TAG values
  • Optimize Docker image of generator
    • use multi-stage build to reduce final image size
    • ensure to use the same Debian version (bookworm) for the builder and final images
    • use --no-install-recommends to reduce unnecessary dependency downloads
    • removed unnecessary unzip dependency in the image
    • use modern syntax (using equal sign) for ENV Dockerfile directive

The first commit addresses the problem that the Docker images for the generator currently always use the latest tagged version in the repository, instead of the latest commit of the branch pointed by the Docker image tag. This comit now allows to build Docker images for the generator using any tag or branch, no matter which tag is latest in the repository.

The most evident issue of the above is that the prom/snmp-generator:main image in Docker Hub (and also the corresponding image in Quay.io) actually contains the generator binary for the latest tag v0.24.1 instead of a binary generated from the latest main branch as suggested by the image tag.

I spent some signifcant time trying to figure out why a newly built image of the generator was unable to process the current generator.yml configuration file in the main branch until I realized that the Dockerfile is fixed to @latest. This PR now allows the generator to work out of the box as shown below.

At the time of this writing, if you try to run make docker-generate, it fails because the generator binary in the built prom/snmp-generator:main image does not understand the latest changes done in the main branch since the latest tag:

$ git status
On branch main
$ cd generator
$ make docker-generate
(...)
docker run -ti -v "/home/hhromic/snmp_exporter/generator:/opt/" "prom/snmp-generator:main" generate
ts=2023-11-11T19:17:56.898Z caller=net_snmp.go:162 level=info msg="Loading MIBs" from=mibs
ts=2023-11-11T19:17:57.127Z caller=main.go:132 level=error msg="Error generating config netsnmp" err="error parsing yml config: yaml: unmarshal errors:\n  line 159: field scale not found in type main.plain\n  line 602: field scale not found in type main.plain\n  line 603: field offset not found in type main.plain"
make: *** [Makefile:86: docker-generate] Error 1

With this PR, the binary in the image is actually built from the main branch and it works as expected:

$ git status
On branch generator-docker-image
$ cd generator
$ make docker-generate REPO_TAG=main
(...)
docker run -ti -v "/home/hhromic/snmp_exporter/generator:/opt/" "prom/snmp-generator:main" generate
ts=2023-11-11T19:20:09.693Z caller=net_snmp.go:175 level=info msg="Loading MIBs" from=mibs
ts=2023-11-11T19:20:09.942Z caller=main.go:53 level=info msg="Generating config for module" module=paloalto_fw
ts=2023-11-11T19:20:09.970Z caller=main.go:68 level=info msg="Generated metrics" module=paloalto_fw metrics=204
(...)
ts=2023-11-11T19:20:11.813Z caller=main.go:93 level=info msg="Config written" file=/opt/snmp.yml

Note: If this PR is merged, it will not be necessary to provide REPO_TAG while in the main branch.

The second commit optimizes the Docker image size by using a multi-stage build and removing unnecessary dependencies.

Image sizes before and after the optimization:

REPOSITORY            TAG       IMAGE ID       CREATED       SIZE
prom/snmp-generator   main      63565e35e294   7 hours ago   931MB

REPOSITORY            TAG       IMAGE ID       CREATED          SIZE
prom/snmp-generator   main      d140116eda2c   38 minutes ago   144MB

I hope you consider these improvements and let me know if you have any concerns.
If you simply don't want to include these changes, feel free to drop this PR. No worries.

@hhromic hhromic mentioned this pull request Nov 18, 2023
Closed
@hhromic
Fix Docker image of generator always pointing to latest tag
b924387 
* new `REPO_TAG` variable can be used to specify the tag/branch to use for a build
* the Makefile default is to use the branch of the current repository `HEAD`
* the Dockerfile default (if building manually with Docker) is `main`
* updated CircleCI configuration to apply the correct `REPO_TAG` values

Signed-off-by: Hugo Hromic <hhromic@gmail.com>
@hhromic
Optimize Docker image of generator
0ba4381 
* use multi-stage build to reduce final image size
* ensure to use the same Debian version (`bookworm`) for the builder and final images
* use `--no-install-recommends` to reduce unnecessary dependency downloads
* removed unnecessary `unzip` dependency in the image
* use modern syntax (using equal sign) for `ENV` Dockerfile directive

Signed-off-by: Hugo Hromic <hhromic@gmail.com>
SuperQ
SuperQ approved these changes Nov 21, 2023
View reviewed changes
Copy link
Member

@SuperQ SuperQ left a comment

Choose a reason for hiding this comment

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

Makes sense, thanks for all the debugging of this build process.

@SuperQ SuperQ merged commit db7cb38 into prometheus:main Nov 21, 2023
2 checks passed
@hhromic hhromic deleted the generator-docker-image branch November 21, 2023 10:53
SuperQ added a commit that referenced this pull request Nov 24, 2023
@SuperQ
Release v0.25.0
864bc26 
* [ENHANCEMENT] generator: Add support for subsequent address family #782
* [ENHANCEMENT] generator: Fix lookups to match OIDs closer to the index OID. #828
* [FEATURE] Add a scaling factor #1026
* [FEATURE] generator: Enable passing input file, output file, and mibs dir as flags #1028
* [FEATURE] Add an offset factor #1029
* [BUGFIX] Fix and optimize generator Docker image building #1045

snmp.yml changes:

* Override `bsnAPName` to DisplayString #660
* Import TP-Link EAP MIB  #833
* Updated Mikrotik neighbor indexes make them unique #986
* Update PowerNet MIB to v4.5.1 #1003
* Refactor HOST-RESOURCES-MIB #1027
* Update keepalived MIB files to latest version #1044

Signed-off-by: SuperQ <superq@gmail.com>
@SuperQ SuperQ mentioned this pull request Nov 24, 2023
Merged
SuperQ added a commit that referenced this pull request Nov 26, 2023
@SuperQ
Release v0.25.0
81d8785 
* [ENHANCEMENT] generator: Add support for subsequent address family #782
* [ENHANCEMENT] generator: Fix lookups to match OIDs closer to the index OID. #828
* [FEATURE] Add a scaling factor #1026
* [FEATURE] generator: Enable passing input file, output file, and mibs dir as flags #1028
* [FEATURE] Add an offset factor #1029
* [BUGFIX] Fix and optimize generator Docker image building #1045

snmp.yml changes:

* Override `bsnAPName` to DisplayString #660
* Import TP-Link EAP MIB  #833
* Updated Mikrotik neighbor indexes make them unique #986
* Update PowerNet MIB to v4.5.1 #1003
* Refactor HOST-RESOURCES-MIB #1027
* Update keepalived MIB files to latest version #1044

Signed-off-by: SuperQ <superq@gmail.com>
SuperQ added a commit that referenced this pull request Dec 6, 2023
@SuperQ
Release v0.25.0
8dafa43 
* [ENHANCEMENT] generator: Add support for subsequent address family #782
* [ENHANCEMENT] generator: Fix lookups to match OIDs closer to the index OID. #828
* [FEATURE] Add a scaling factor #1026
* [FEATURE] generator: Enable passing input file, output file, and mibs dir as flags #1028
* [FEATURE] Add an offset factor #1029
* [BUGFIX] Fix and optimize generator Docker image building #1045

snmp.yml changes:

* Override `bsnAPName` to DisplayString #660
* Import TP-Link EAP MIB  #833
* Updated Mikrotik neighbor indexes make them unique #986
* Update PowerNet MIB to v4.5.1 #1003
* Refactor HOST-RESOURCES-MIB #1027
* Update keepalived MIB files to latest version #1044

Signed-off-by: SuperQ <superq@gmail.com>
SuperQ added a commit that referenced this pull request Dec 10, 2023
@SuperQ
Release v0.25.0 (#1052)
ea9e638 
* [ENHANCEMENT] generator: Add support for subsequent address family #782
* [ENHANCEMENT] generator: Fix lookups to match OIDs closer to the index OID. #828
* [FEATURE] Add a scaling factor #1026
* [FEATURE] generator: Enable passing input file, output file, and mibs dir as flags #1028
* [FEATURE] Add an offset factor #1029
* [BUGFIX] Fix and optimize generator Docker image building #1045

snmp.yml changes:

* Override `bsnAPName` to DisplayString #660
* Import TP-Link EAP MIB  #833
* Updated Mikrotik neighbor indexes make them unique #986
* Update PowerNet MIB to v4.5.1 #1003
* Refactor HOST-RESOURCES-MIB #1027
* Update keepalived MIB files to latest version #1044

Signed-off-by: SuperQ <superq@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SuperQ SuperQ SuperQ approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@hhromic @SuperQ