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

Optimise builds #274

Merged
merged 10 commits into from
May 15, 2020
Merged

Optimise builds #274

merged 10 commits into from
May 15, 2020

Conversation

tobiasge
Copy link
Member

@tobiasge tobiasge commented Apr 9, 2020

New Behavior

  • More builds are done in parallel
  • Builds are only run when necessary (changes in Netbox, Netboc-docker, Python image)

Contrast to Current Behavior

  • See above

Discussion: Benefits and Drawbacks

When following the "latest" or "v2.7" tags in Kubernetes or Openshift the containers were deployed everytime the GH Action ran. From now on the images are build only when something changed. So the automatic deployment is only triggered when really needed.
The image count was also reduced. Changes to the master branch in Netbox are only made when a new release is ready. So we don't need to build extra images for the latest tag. This tag now follows the newest version of Netbox.

Changes to the Wiki

  • "Build" page: Adapt the list of build scripts

Proposed Release Note Entry

  • Maintenance: Improved build system

Double Check

  • I have read the comments and followed the PR template.
  • I have explained my PR according to the information in the comments.
  • My PR targets the develop branch.

This checks if the source materials (python image, Netbox commit,
netbox-docker commit) have changed since the last build. This check is done
by comparing the digest and commit ids from the previous image with the
given tag to the current values taken from the Git and Docker repositories.

The checks are only performed for builds by the automated builds on Github.
The old version of "build-branches.sh" skipped the pushing of images when one of
them didn't change its sources. When looking at the Netbox branches I noticed
that the "master" branch only changes when there is a new release. Because we
build the new releases anyway in "build-latest.sh" that leaves "develop" and
"develop-*" which change regularly. The new script "build-next.sh" is responsible
for building "develop-*" as the next major release of Netbox. The build of
"develop" is moved to the Github Action build matrix. This has the additional
advantage of being faster because more builds are done in parallel.
Latest tag is now added to the new version when it is release by Netbox.
We are no longer building the images with Travis CI.
The variable for the latest tag didn't contain all the needed values.
The DOCKER_FROM is set to an empty value in the push tests.
This expands the check to catch this test case
@cimnine
Copy link
Collaborator

cimnine commented Apr 9, 2020

When I read through this, I start to think we should rewrite the build script in something else than bash, Python or so 🙈

@tobiasge
Copy link
Member Author

I think for now Bash is OK, but if we want to add more functionality we should do that.

Build now prints a message when no devlop-* branch is found instead of crashing
@tobiasge
Copy link
Member Author

The builds in my repository with the new script have been stable for a while now. I think we can merge this.

@tobiasge tobiasge requested a review from cimnine April 24, 2020 06:35
Copy link
Collaborator

@cimnine cimnine left a comment

Choose a reason for hiding this comment

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

Did you run shellcheck against shell files?

Dockerfile Show resolved Hide resolved
build.sh Outdated Show resolved Hide resolved
build.sh Show resolved Hide resolved
@tobiasge
Copy link
Member Author

Did you run shellcheck against shell files?

I did. But I found just found one more shellcheck problem. Will commit a fix to this branch.

tobiasge and others added 2 commits May 14, 2020 13:49
Include suggestion from cimnine

Co-authored-by: Christian Mäder <cimnine@users.noreply.github.com>
Copy link
Collaborator

@cimnine cimnine left a comment

Choose a reason for hiding this comment

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

I still don't understand the whole file, but since it seems working in your branch, let's go for it.

@cimnine cimnine added this to the 0.24.0 milestone May 14, 2020
@cimnine
Copy link
Collaborator

cimnine commented May 15, 2020

I believe we must update the repository settings shortly after this is merged to specify which of the new jobs are required to run and pass in order for a commit to get 'green'.

@tobiasge tobiasge merged commit a6584d2 into netbox-community:develop May 15, 2020
@tobiasge tobiasge mentioned this pull request May 15, 2020
3 tasks
@tobiasge tobiasge deleted the optimise-builds branch August 24, 2020 06:43
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.

None yet

2 participants