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

fix: Split the CI workflow into ci-test and ci to eliminate redundant work… #2770

Merged
merged 3 commits into from
Nov 1, 2022
Merged

fix: Split the CI workflow into ci-test and ci to eliminate redundant work… #2770

merged 3 commits into from
Nov 1, 2022

Conversation

spring1843
Copy link
Contributor

@spring1843 spring1843 commented Nov 1, 2022

… and speed up testing feedback

Fixes #

Description

  • Separate CI activities (license check, lint and etc..) other than testing into a different workflow, these activities need to happen only once whereas before they were happening multiple times on each k8s version matrix (5 times) which was prolonging test execution
  • Separate CI tests into a different workflow so that they can start to execute in parallel and immediately without having to wait for other checks, allowing them to finish quicker and notify the user faster
  • Upload coverage data to coveralls only once, previously this was happening on each
  • Fix formatting for conditions in release-tag.yml
  • No logical change to make ci to preserve the same dev experience and limit the changes to Workflows

How was this change tested?

  • Different fork

Does this change impact docs?

  • Yes, PR includes docs updates
  • Yes, issue opened: #
  • No

Release Note


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@spring1843 spring1843 requested a review from a team as a code owner November 1, 2022 06:09
@netlify
Copy link

netlify bot commented Nov 1, 2022

Deploy Preview for karpenter-docs-prod canceled.

Name Link
🔨 Latest commit 6017788
🔍 Latest deploy log https://app.netlify.com/sites/karpenter-docs-prod/deploys/636159f44992af0008ef3c91

@spring1843 spring1843 enabled auto-merge (squash) November 1, 2022 06:19
- uses: shogo82148/actions-goveralls@v1
if: env.K8S_VERSION == '1.24.x'
Copy link
Contributor

Choose a reason for hiding this comment

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

Why restrict this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because otherwise it's going to do it as many times (5 times), while this needs to happen onyly once

Copy link
Contributor

Choose a reason for hiding this comment

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

We're going to need to constantly maintain this whenever we change Kubernetes versions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We only need to change it if we get rid of 1.24.x I'll have to figure out how to access the dimensions from here and reference the last one instead of this to avoid that.

ci: toolchain verify licenses vulncheck battletest coverage ## Run all steps used by continuous integration
ci-test: battletest coverage ## Runs tests and submits coverage

ci-non-test: verify licenses vulncheck ## Runs checks other than tests
Copy link
Contributor

Choose a reason for hiding this comment

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

Trying to think of more idiomatic names for this. I'd like to make sure we share as much as possible between karpenter and karpenter-core.

I like this separation, since it enables developers to run ci-test locally without significant time investment of the license/vuln checks. I think we might be able to collapse battletest and test, since I'm not sure it actually takes very much more time

presubmit Runs codegen, linting, testing, coverage, etc. It's fast
presubmit-slow Runs license checks, vulnerabilities, slow stuff.

This makes me think that we might actually want to run the slow stuff on postsubmit. These fail so rarely. It would be really nice to dramatically improve our merge times.

That would leave us with

presubmit Devs run before submission, Runs before CI checks. SLA <5min, ideally 3m?
postsubmit GH runs after submission, may take longer

Happy to chat offline

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Those are good idea, we also don't really need vulnerability scanning right here as we are doing it in another workflow. I'll do them in a follow up PR if you don't mind.

@spring1843 spring1843 merged commit 9dea20a into aws:main Nov 1, 2022
@spring1843 spring1843 deleted the rm/split-ci-workflows branch November 2, 2022 21:03
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