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

Require cloud build to succeed in make dist checks. #9218

Merged
merged 3 commits into from Aug 3, 2020

Conversation

Ferroin
Copy link
Member

@Ferroin Ferroin commented May 29, 2020

Summary

This updates our make dist check to only pass if building with Netdata Cloud functionality succeeds.

Component Name

area/ci

Test Plan

CI check passes on this PR.

Additional Information

Fixes: #8284

@squash-labs
Copy link

squash-labs bot commented May 29, 2020

Manage this branch in Squash

Test this branch here: https://ferroinrobust-dist-check-zf8pj.squash.io

prologic
prologic previously approved these changes Jun 1, 2020
Copy link
Contributor

@prologic prologic left a comment

Choose a reason for hiding this comment

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

LGTM. Just a thought though...

Should we refactor .github/scripts/run_install_with_dist_file.sh to accept arguments we can pass in from CI and have a few different things we can check? Not just assume that all checks will now ensure the Cloud functionality builds correctly, but other optional features as separate jobs?

@Ferroin
Copy link
Member Author

Ferroin commented Jun 1, 2020

Should we refactor .github/scripts/run_install_with_dist_file.sh to accept arguments we can pass in from CI and have a few different things we can check? Not just assume that all checks will now ensure the Cloud functionality builds correctly, but other optional features as separate jobs?

I'd been thinking about this myself, but a lot more generically than just that.

@Ferroin
Copy link
Member Author

Ferroin commented Jun 11, 2020

@prologic Could you possibly take a look into the failures here, I'm stumped. It looks like for some reason install-required-packages.sh netdata isn't pulling in cmake, but only in this particular GHA instance (if it weren't anywhere else, other checks would be failing and users would be complaining).

@prologic
Copy link
Contributor

@Ferroin You need to see what the dist build failures are. The CentOS 8.x failures are a KP and I'm working on them. Please have a look :)

@Ferroin Ferroin force-pushed the robust-dist-check branch 3 times, most recently from bb336e4 to 101ff04 Compare June 24, 2020 17:25
@prologic
Copy link
Contributor

Dist check is still failing. Looks like missing wget command?

The default behavior in `kickstart.sh` is to run
`install-required-packages.sh` with just the `netdata` package set, and
we don't even document how to make it run with `netdata-all`, so a vast
majority of users will only be running using the `netdata` package set.
We should be testing our distfiles against our current version of
`install-required-packages.sh`, not some pre-generated test image that
we have to manage ina separaterepo.

Also, disable telemetry for the test build so we don't accidentally
pollute the stats.
@Ferroin Ferroin requested a review from amoss as a code owner July 30, 2020 17:15
@Ferroin Ferroin requested review from prologic and removed request for cosmix July 30, 2020 17:21
@Ferroin
Copy link
Member Author

Ferroin commented Jul 30, 2020

@prologic Finally got around to updating this and getting it working correctly, if you could take another look at it that would be appreciated.

@prologic prologic merged commit 5f160be into netdata:master Aug 3, 2020
@prologic
Copy link
Contributor

prologic commented Aug 3, 2020

Merged on behalf of @Ferroin (while away on vacation); as this directly impacts our CI and improves it.

@Ferroin Ferroin deleted the robust-dist-check branch August 10, 2020 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add more robust checking of make dist.
2 participants