-
Notifications
You must be signed in to change notification settings - Fork 907
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
chore: remove unuse ghaction step and collect logs of karmadactl test. #4824
Conversation
@zhzhuang-zju Please take a look |
/assign |
# Only upload logs on failure. | ||
if: ${{ failure() }} | ||
- name: export logs | ||
if: always() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it necessary to collect logs when workflow succeeds or is canceled as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hope it can be treated the same as CI,
karmada/.github/workflows/ci.yml
Lines 157 to 159 in 4898c0f
- name: upload logs | |
if: always() | |
uses: actions/upload-artifact@v3 |
@liangyuanpeng This PR consists of two parts actually as you mentioned in the PR description:
BTW, thanks for this commit~ |
Good catch! seems like it's provided by github action or some github action cache. i will update the
I am wroting the code for #4801 and I was not satisfied with the workflow. He increased the number of parallel runs of the action, so i more research this workflow and found it. |
In the next PR, I will continue to update this workflow, use one action to build the image and karmadactl, and then test it in other actions, CI workflow is the same thing, and let's talk about it in other space. |
There seems to have been a discussion about it before,#3168 (comment) @RainbowMango
Very welcome. If you make any progress, please ping me~ |
d7c4dc2
to
731aa57
Compare
731aa57
to
4b9b9c9
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #4824 +/- ##
==========================================
+ Coverage 29.41% 29.67% +0.26%
==========================================
Files 632 632
Lines 43835 43901 +66
==========================================
+ Hits 12892 13027 +135
+ Misses 30003 29931 -72
- Partials 940 943 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
4b9b9c9
to
3b59b69
Compare
rebased main /assign @zhzhuang-zju |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
others LGTM
Signed-off-by: Lan Liang <[email protected]>
3b59b69
to
afc62fe
Compare
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: zhzhuang-zju The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest |
What type of PR is this?
What this PR does / why we need it:
/kind cleanup
this CI is using the kubernetes from created by
cli-testing-environment
and have not usedhelm/kind-action
, so juse remove it..karmada/hack/cli-testing-environment.sh
Lines 53 to 55 in 4898c0f
the second change is collect logs for kind kubernetes.
/assign @RainbowMango
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?: