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

update verify pipeline and omnibus build/test to use containers #13489

Merged
merged 36 commits into from Jan 23, 2023

Conversation

evanahlberg
Copy link
Contributor

@evanahlberg evanahlberg commented Jan 9, 2023

Signed-off-by: Evan Ahlberg evanahlberg@gmail.com

Description

Update verify and omnibus release pipelines to use containers

The following will happen when we merge this PR:

  • verify pipeline will be replaced with the new pipeline that uses containers
  • verify/adhoc pipeline will be created
  • verify/adhoc-canary pipeline will be created
  • verify/release pipeline will be created

Future PRs will run the new verify pipeline

While we test the new pipelines and incrementally move over to them, the omnibus/release pipeline will still run on merges to main for now.

We needed the new pipelines to be created so we can investigate esoteric builds, uploading to artifactory, and test the verify/adhoc pipelines.

Related Issue

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Chore (non-breaking change that does not add functionality or fix an issue)

Checklist:

  • I have read the CONTRIBUTING document.
  • I have run the pre-merge tests locally and they pass.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • All commits have been signed-off for the Developer Certificate of Origin.

evanahlberg and others added 10 commits January 9, 2023 11:04
Signed-off-by: Evan Ahlberg <evanahlberg@gmail.com>
Signed-off-by: Evan Ahlberg <evanahlberg@gmail.com>
Signed-off-by: Evan Ahlberg <evanahlberg@gmail.com>
Signed-off-by: Evan Ahlberg <evanahlberg@gmail.com>
Signed-off-by: Evan Ahlberg <evanahlberg@gmail.com>
Signed-off-by: Evan Ahlberg <evanahlberg@gmail.com>
Signed-off-by: Evan Ahlberg <evanahlberg@gmail.com>
Signed-off-by: Evan Ahlberg <evanahlberg@gmail.com>
Signed-off-by: Evan Ahlberg <evanahlberg@gmail.com>
Signed-off-by: Gregory Schofield <grschofi@progress.com>
Gregory Schofield and others added 7 commits January 12, 2023 11:05
Signed-off-by: Gregory Schofield <grschofi@progress.com>
Signed-off-by: Gregory Schofield <grschofi@progress.com>
Signed-off-by: Gregory Schofield <grschofi@progress.com>
Signed-off-by: Gregory Schofield <grschofi@progress.com>
@gcs-devel gcs-devel force-pushed the evanahlberg/BS-125 branch 4 times, most recently from 6425698 to 3f2c985 Compare January 16, 2023 15:33
evanahlberg and others added 6 commits January 16, 2023 12:44
Signed-off-by: Evan Ahlberg <evanahlberg@gmail.com>
Signed-off-by: Evan Ahlberg <evanahlberg@gmail.com>
Signed-off-by: Jesse Prieur <jesse.prieur@gmail.com>
Signed-off-by: Jesse Prieur <jesse.prieur@gmail.com>
jesseprieur and others added 9 commits January 17, 2023 11:54
Updating Dynamic Pipeline To Use Heredocs
Signed-off-by: Evan Ahlberg <evanahlberg@gmail.com>
Signed-off-by: Evan Ahlberg <evanahlberg@gmail.com>
Signed-off-by: Evan Ahlberg <evanahlberg@gmail.com>
move omnibus test and build to own file and create ad hoc pipeline
Signed-off-by: Evan Ahlberg <evanahlberg@gmail.com>
Signed-off-by: Evan Ahlberg <evanahlberg@gmail.com>
@evanahlberg evanahlberg marked this pull request as ready for review January 18, 2023 19:10
@evanahlberg evanahlberg requested review from a team as code owners January 18, 2023 19:10
Copy link
Contributor

@johnmccrae johnmccrae left a comment

Choose a reason for hiding this comment

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

After reviewing the video and walking the code I can really see how this can be better.

Copy link
Contributor

@tpowell-progress tpowell-progress left a comment

Choose a reason for hiding this comment

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

Feels like switching to bash HEREDOCs will make this more maintainable and readable.

Still reviewing the rest of the PR

.buildkite/build-test-omnibus.sh Show resolved Hide resolved
.buildkite/verify.pipeline.sh Show resolved Hide resolved
.buildkite/verify.pipeline.sh Show resolved Hide resolved
.buildkite/verify.pipeline.sh Show resolved Hide resolved
.buildkite/verify.pipeline.sh Show resolved Hide resolved
.buildkite/verify.pipeline.sh Show resolved Hide resolved
@gcs-devel
Copy link
Contributor

@tpowell-progress We did implement the pipeline using heredoc on a branch but we found a few downsides in using it.

  1. You need to indent using tabs (not spaces) which is problematic when people (like me) use spaces instead of tabs in their text editor.
  2. If you pipe the heredoc to sed to handle the indentation, then the heredoc isn't "what you see is what you get" and you have to figure out how many spaces to indent and pass that to sed
  3. It starts to get weird when if statements and case statements are mixed in.

I agree it is more readable but these are the reasons why we stuck with echo for now. In the future it would be nice to refactor this into a ruby script using ruby heredoc or erb. We actually tried that too but ruby isn't available on the agent outside the container at the moment.

Gregory Schofield added 2 commits January 20, 2023 15:37
Signed-off-by: Gregory Schofield <grschofi@progress.com>
Signed-off-by: Gregory Schofield <grschofi@progress.com>
Signed-off-by: Gregory Schofield <grschofi@progress.com>
@sonarcloud
Copy link

sonarcloud bot commented Jan 20, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Collaborator

@neha-p6 neha-p6 left a comment

Choose a reason for hiding this comment

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

Do we have any plan for Mac and AIX systems?

@tpowell-progress tpowell-progress merged commit b129100 into main Jan 23, 2023
@tpowell-progress tpowell-progress deleted the evanahlberg/BS-125 branch January 23, 2023 18:10
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

6 participants