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

Fixed double-build of TR RPMs in CiaB makefile #3883

Merged
merged 1 commit into from
Oct 9, 2019

Conversation

ocket8888
Copy link
Contributor

@ocket8888 ocket8888 commented Aug 20, 2019

What does this PR (Pull Request) do?

  • This PR is not related to any Issue

When building packages for CDN-in-a-Box using make with multiple "jobs" (i.e. with -j), there is a race condition building the Traffic Router and Tomcat RPMs, since one build process creates both and they both depend on changes to the TR source. In cases where this manifested - which is often - the build will either fail or simply do double the work it needs to do when building those RPMs. This PR eliminates that by changing the rule to a pattern-based one, which make will now properly recognize as a single output (consisting of multiple files) of a single build process.

Which Traffic Control components are affected by this PR?

  • CDN in a Box

No docs because it's a bugfix

What is the best way to verify this PR?

Build the RPMs for CDN-in-a-Box using make with the -j flag, observe that it consistently has no issue creating RPMs for both Tomcat and Traffic Router exactly once (note that in order to do that, the build_default Docker network must already exist - which is accomplished by just having built an RPM on the same machine before - as otherwise duplicate networks will be created).

No tests because this is just the build system for a toy environment.

If this is a bug fix, what versions of Traffic Control are affected?

N/A - CDN-in-a-Box is not a supported component of Traffic Control, and no such support is purported to exist in any existing releases.

The following criteria are ALL met by this PR

  • I have explained why tests are unnecessary
  • I have explained why documentation is unnecessary
  • An update to CHANGELOG.md is not necessary
  • This PR includes any and all required license headers
  • This PR does not include a database migration
  • This PR DOES NOT FIX A SERIOUS SECURITY VULNERABILITY

@ocket8888 ocket8888 added bug something isn't working as intended low impact affects only a small portion of a CDN, and cannot itself break one cdn-in-a-box related to the Docker-based CDN-in-a-Box system labels Aug 20, 2019
@asfgit
Copy link
Contributor

asfgit commented Aug 20, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/4175/
Test PASSed.

@mitchell852
Copy link
Member

maybe @mhoppa can review this?

@mhoppa
Copy link
Contributor

mhoppa commented Sep 26, 2019

It looks great - let me pull down and test thought just to make sure it doesn't do anything funky it shouldnt

Copy link
Contributor

@mhoppa mhoppa left a comment

Choose a reason for hiding this comment

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

Test locally and works for me

@mitchell852 mitchell852 merged commit 87444f9 into apache:master Oct 9, 2019
@ocket8888 ocket8888 deleted the ciab-makefile branch October 10, 2019 00:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something isn't working as intended cdn-in-a-box related to the Docker-based CDN-in-a-Box system low impact affects only a small portion of a CDN, and cannot itself break one
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants