Skip to content

skip all pipelines based on what is in the PR#6996

Merged
cwayne18 merged 3 commits into
k3s-io:masterfrom
matttrach:drone-skip-files-6980
Mar 14, 2023
Merged

skip all pipelines based on what is in the PR#6996
cwayne18 merged 3 commits into
k3s-io:masterfrom
matttrach:drone-skip-files-6980

Conversation

@matttrach

Copy link
Copy Markdown
Contributor

Proposed Changes

This change will add a pipeline which implements drone-skip-pipeline.
The purpose is to provide some mechanism to automatically skip pipelines which are not necessary due to the files that are being changed. For example, if a markdown file is the only file changed, then we don't need to run functional tests. This will reduce our integration time.

Types of Changes

No new code to k3s, all changes in .drone.yml

Verification

Unfortunately, Drone provides no way to test this outside of letting it run in your repo.

Linked Issues

This addresses #6980.

User-Facing Change

This has no user facing change.

Further Comments

There is another plugin which may give us similar functionality, but is out of date and requires configuration on the drone server, I would like to try this one first to avoid the additional handoffs.
There is more than one way to implement this plugin, the way I have provided is the one with the fewest changes to existing steps, I am open to additional suggestions.

@matttrach matttrach requested a review from a team as a code owner February 21, 2023 14:46
@matttrach matttrach self-assigned this Feb 21, 2023
@codecov-commenter

codecov-commenter commented Feb 28, 2023

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 9.65%. Comparing base (ea094d1) to head (52e532d).
Report is 724 commits behind head on master.

Additional details and impacted files
@@          Coverage Diff           @@
##           master   #6996   +/-   ##
======================================
  Coverage    9.65%   9.65%           
======================================
  Files         146     146           
  Lines       10738   10738           
======================================
  Hits         1037    1037           
  Misses       9483    9483           
  Partials      218     218           
Flag Coverage Δ
unittests 9.65% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@brandond brandond left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not sure what's going on with all the commits that have been added to this branch after approval; please get it back to just the single change before merging.

@matttrach

Copy link
Copy Markdown
Contributor Author

sorry, I will. I was finally able to test this and it doesn't work... :(
The plugin is only compiled on amd64, I think I will need to head back to the drawing board on this one.

@matttrach matttrach marked this pull request as draft March 2, 2023 19:09
@matttrach

Copy link
Copy Markdown
Contributor Author

converting this to a draft so I don't spam reviewers

@matttrach matttrach force-pushed the drone-skip-files-6980 branch from 340e8fd to 85a0ea5 Compare March 7, 2023 18:55
@matttrach

Copy link
Copy Markdown
Contributor Author

I put everything into a single commit, I believe this is ready for review.

Comment thread .drone.yml
volumes:
- name: docker
host:
path: /var/run/docker.sock

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

the diff is unclear here, but if you look at the file I just removed some spacing here to conform to the way the rest of the yaml is written (lists have same spacing as parent)

Comment thread README.md

Security issues in K3s can be reported by sending an email to [security@k3s.io](mailto:security@k3s.io). Please do not file issues about security issues.
Security issues in K3s can be reported by sending an email to [security@k3s.io](mailto:security@k3s.io).
Please do not file issues about security issues.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is a trivial spacing change to validate that .md files are ignored.

@matttrach matttrach force-pushed the drone-skip-files-6980 branch from 85a0ea5 to 6148ff1 Compare March 7, 2023 20:11
…drone config to use droneignore to skip CI when files are all matched

Signed-off-by: matttrach <matttrach@gmail.com>
@matttrach matttrach force-pushed the drone-skip-files-6980 branch from 865e5b0 to 00993dc Compare March 7, 2023 20:37
@matttrach matttrach marked this pull request as ready for review March 7, 2023 20:38

@cwayne18 cwayne18 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks neat thanks! Can we add .github/*, MAINTAINERS, CODEOWNERS to the skip list please?

… list

Signed-off-by: matttrach <matttrach@gmail.com>
@matttrach

Copy link
Copy Markdown
Contributor Author

done. Also added DCO, channel.yaml, and LICENSE (although they are unlikely to change)

@matttrach matttrach requested a review from cwayne18 March 8, 2023 16:12

@dereknola dereknola left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is is possible to put all the repeated bash commands into a script file and just call that?

@matttrach

Copy link
Copy Markdown
Contributor Author

I think the special exit code has to be executed from the root scope, I can probably add a file and execute it, but it would only reduce 1 line of code per copy.

@brandond brandond left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we want this on all CI runs, or just on PR? It feels like we should never skip running CI on merges or tags - basically anything that runs on drone-publish. Should we add something to restrict this to just pull branches on drone-pr?

trigger:
  instance:
  - drone-pr.k3s.io
  ref:
  - refs/pull/*/head

Signed-off-by: matttrach <matttrach@gmail.com>
@matttrach

Copy link
Copy Markdown
Contributor Author

Definitely don't wanna skip on tags, thanks!
I think bounding the skip to pushes and pull requests should give us a good compromise for how often.

@ivan-claire ivan-claire left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks good to me.

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.

7 participants