Skip to content

[improve][ci] Improve CI ssh access in forks, don't fail build if setting up ssh access fails - #19127

Merged
tisonkun merged 1 commit into
apache:masterfrom
lhotari:lh-improve-ci-ssh-access-in-forks
Jan 5, 2023
Merged

[improve][ci] Improve CI ssh access in forks, don't fail build if setting up ssh access fails#19127
tisonkun merged 1 commit into
apache:masterfrom
lhotari:lh-improve-ci-ssh-access-in-forks

Conversation

@lhotari

@lhotari lhotari commented Jan 4, 2023

Copy link
Copy Markdown
Member

Motivation

The ssh access setup step that is part of Pulsar CI can fail in builds that are run in forks. The ssh access feature is active in CI builds run in forks to allow connecting to the running build with ssh for debugging purposes.

This is the ssh access setup step:
https://github.com/apache/pulsar/blob/master/.github/workflows/pulsar-ci.yaml#L88-L93

Sometime this fails with an error message "flag needs an argument: --admin-socket". Example of failure:
https://github.com/michaeljmarshall/pulsar/actions/runs/3833202322/jobs/6527659487#step:4:150

Modifications

  • wait up to 10 seconds for admin socket to appear
  • add "continue-on-error: true" to ssh setup step so that errors don't prevent the build from continuing

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository: lhotari#122

…ting up ssh access fails

- address error message "flag needs an argument: --admin-socket"
  - wait up to 10 seconds for admin socket to appear
- add "continue-on-error: true" to ssh setup step so that errors don't prevent the build from continuing
@lhotari
lhotari force-pushed the lh-improve-ci-ssh-access-in-forks branch from 0ca1168 to d56a9c1 Compare January 4, 2023 07:05
@lhotari

lhotari commented Jan 4, 2023

Copy link
Copy Markdown
Member Author

/pulsarbot rerun-failure-checks

@codecov-commenter

codecov-commenter commented Jan 4, 2023

Copy link
Copy Markdown

Codecov Report

Merging #19127 (d56a9c1) into master (9ec1d07) will decrease coverage by 0.38%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #19127      +/-   ##
============================================
- Coverage     47.73%   47.34%   -0.39%     
+ Complexity    10819    10716     -103     
============================================
  Files           712      712              
  Lines         69645    69645              
  Branches       7481     7481              
============================================
- Hits          33242    32975     -267     
- Misses        32699    32963     +264     
- Partials       3704     3707       +3     
Flag Coverage Δ
unittests 47.34% <ø> (-0.39%) ⬇️

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

Impacted Files Coverage Δ
.../pulsar/broker/service/SharedConsumerAssignor.java 3.70% <0.00%> (-64.82%) ⬇️
...rvice/schema/KeyValueSchemaCompatibilityCheck.java 21.62% <0.00%> (-45.95%) ⬇️
...apache/pulsar/broker/service/EntryAndMetadata.java 0.00% <0.00%> (-40.75%) ⬇️
...r/service/AbstractDispatcherMultipleConsumers.java 56.45% <0.00%> (-14.52%) ⬇️
.../apache/pulsar/broker/admin/impl/PackagesBase.java 54.12% <0.00%> (-13.77%) ⬇️
...e/pulsar/broker/service/EntryBatchIndexesAcks.java 82.14% <0.00%> (-10.72%) ⬇️
...sistent/PersistentDispatcherMultipleConsumers.java 51.96% <0.00%> (-7.18%) ⬇️
...ersistentStickyKeyDispatcherMultipleConsumers.java 54.41% <0.00%> (-5.89%) ⬇️
...pulsar/broker/service/PulsarCommandSenderImpl.java 67.85% <0.00%> (-5.62%) ⬇️
...e/HashRangeExclusiveStickyKeyConsumerSelector.java 36.92% <0.00%> (-3.08%) ⬇️
... and 37 more

@lhotari
lhotari requested a review from tisonkun January 5, 2023 08:01

@michaeljmarshall michaeljmarshall 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.

LGTM

@tisonkun tisonkun 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.

LGTM.

@tisonkun

tisonkun commented Jan 5, 2023

Copy link
Copy Markdown
Member

Merging...

@tisonkun
tisonkun merged commit 4028ad3 into apache:master Jan 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/ci doc-not-needed Your PR changes do not impact docs ready-to-test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants