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.

Sign up for GitHub

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

Jump to bottom

Fix failing k8s action in the workflow #70

Merged
merged 24 commits into from
Jan 13, 2023
Merged

Fix failing k8s action in the workflow #70

merged 24 commits into from
Jan 13, 2023

Conversation

alphaX86
Copy link
Member

Description

This PR fixes #68

Notes for Reviewers
WIP and test required

Signed commits

  • Yes, I signed my commits.

@alphaX86
Change action version
0fe1b1f 
Signed-off-by: Aadhitya A <[email protected]>
@alphaX86
Change GH action
7b92619 
Signed-off-by: Aadhitya A <[email protected]>
@alphaX86 alphaX86 added area/ci Continuous integration | Build and release area/lifecycle Lifecycle management (install, uninstall, configure) related pr/do-not-merge PRs not ready to be merged labels Nov 15, 2022
@alphaX86 alphaX86 changed the title Cluster test Fix failing k8s action in the workflow Nov 15, 2022
@alphaX86
Copy link
Member Author

alphaX86 commented Nov 15, 2022 •
edited
Loading

Action is being made to run with simultaneous changes made in this branch. If worked, will ping for review

@alphaX86
Revert action on self-hosted runners
b80646b 
Signed-off-by: Aadhitya A <[email protected]>
@alphaX86
Make changes
60ab06e 
Signed-off-by: Aadhitya A <[email protected]>
@alphaX86
Edit workflow for testing
af1fc43 
Signed-off-by: Aadhitya A <[email protected]>
@alphaX86
Edit permission in minikube
e364dcc 
Signed-off-by: Aadhitya A <[email protected]>
@alphaX86
Edit script
25510b1 
Signed-off-by: Aadhitya A <[email protected]>
@alphaX86
Correct minor bugs in bash scripts
b182a1a 
Signed-off-by: Aadhitya A <[email protected]>
@alphaX86
Correct minor mistake
44450a4 
Signed-off-by: Aadhitya A <[email protected]>
@alphaX86
Edit memory size
683d040 
Signed-off-by: Aadhitya A <[email protected]>
@alphaX86 alphaX86 removed the pr/do-not-merge PRs not ready to be merged label Nov 17, 2022
@hershd23
Copy link
Contributor

Can you send the link to a local workflow run?

hershd23
hershd23 reviewed Nov 18, 2022
View reviewed changes
Copy link
Contributor

@hershd23 hershd23 left a comment

Choose a reason for hiding this comment

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

There are 3 different ways we are setting up kubernetes in this PR

manusa/actions-setup-minikube
medyagh/[email protected]
And a manual curl get and install.

What is the problem with each of the methods exactly? Can we not use one method consistently throughout the workflows?

@alphaX86
Copy link
Member Author

There are 3 different ways we are setting up kubernetes in this PR

manusa/actions-setup-minikube medyagh/[email protected] And a manual curl get and install.

What is the problem with each of the methods exactly? Can we not use one method consistently throughout the workflows?

OK so the main issue is present in the self-hosted runner where the k8s Workflow didn't work during setup and the action throws an error saying there's no permission for the action to unzip files during setup. This is why for self-hosted runners alone, I used curl based method and eventually it worked rather than using other 2 k8s action which throws several access errors.

For normal ones, I just changed them for testing which is reliable. Don't get alarmed 😅

@alphaX86
Copy link
Member Author

alphaX86 commented Nov 19, 2022 •
edited
Loading

Can you send the link to a local workflow run?

All workflows are made to run here itself in this repo under cluster-test branch, so the logs are present in the Actions tab (since Self-hosted runners needs valid credentials for accessing it and forked repos don't have it)

@gyohuangxin
Copy link
Member

gyohuangxin commented Nov 22, 2022 •
edited
Loading

@alphaX86 Excellent! It seems your changes to kubernetes worked: https://github.com/layer5io/meshery-smp-action/actions/runs/3472699467/jobs/5803912095, so please revise this PR based on @hershd23 's comments.

@alphaX86
Make changes in workflow schema
bfb5de3 
Signed-off-by: Aadhitya A <[email protected]>
@alphaX86
Make changes
b70ce73 
Signed-off-by: Aadhitya A <[email protected]>
@alphaX86
Copy link
Member Author

Logs:
https://github.com/layer5io/meshery-smp-action/actions/runs/3573114008/jobs/6006793795

https://github.com/layer5io/meshery-smp-action/actions/runs/3573116243

@leecalcote
Copy link
Member

leecalcote commented Nov 29, 2022 •
edited
Loading

/home/runner/work/_actions/layer5io/meshery-smp-action/master/mesheryctl.sh: line 33: adapters: bad array subscript
1206

@gyohuangxin
Copy link
Member

/home/runner/work/_actions/layer5io/meshery-smp-action/master/mesheryctl.sh: line 33: adapters: bad array subscript 1206

@alphaX86 Are you looking at this error or do you need some help?

@alphaX86
Edit minor change
39caea7 
Signed-off-by: Aadhitya A <[email protected]>
@alphaX86
Copy link
Member Author

alphaX86 commented Dec 6, 2022

/home/runner/work/_actions/layer5io/meshery-smp-action/master/mesheryctl.sh: line 33: adapters: bad array subscript 1206

@alphaX86 Are you looking at this error or do you need some help?

Looking for now, got my laptop recently fixed after a week 😅

@gyohuangxin
Copy link
Member

@alphaX86 Hi, do you have any update?

@alphaX86
Copy link
Member Author

alphaX86 commented Jan 2, 2023

Uh oh! 🙆‍♂️ totally forgot this... I'll resume this work on Next week Mon and update you back... Until then, this PR is now a draft

@alphaX86 alphaX86 marked this pull request as draft January 2, 2023 15:48
@alphaX86 alphaX86 added the issue/willfix This issue will be worked on label Jan 2, 2023
@gyohuangxin
Copy link
Member

@alphaX86 Good, it's ok. Thank you!

@alphaX86
Add debug print for check logs
79ae385 
Signed-off-by: Aadhitya A <[email protected]>
@alphaX86
Make changes
b554ea3 
Signed-off-by: Aadhitya A <[email protected]>
@alphaX86 alphaX86 marked this pull request as ready for review January 10, 2023 16:12
gyohuangxin
gyohuangxin approved these changes Jan 11, 2023
View reviewed changes
Copy link
Member

@gyohuangxin gyohuangxin left a comment

Choose a reason for hiding this comment

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

Thanks for continuing to fix this issue, lgtm.

@gyohuangxin gyohuangxin requested a review from hershd23 January 11, 2023 15:37
hershd23
hershd23 approved these changes Jan 13, 2023
View reviewed changes
Copy link
Contributor

@hershd23 hershd23 left a comment

Choose a reason for hiding this comment

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

LGTM

@gyohuangxin
Copy link
Member

@hershd23 Thanks.

@gyohuangxin gyohuangxin merged commit d8db463 into master Jan 13, 2023
@gyohuangxin gyohuangxin deleted the cluster-test branch January 13, 2023 06:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hershd23 hershd23 hershd23 approved these changes

@gyohuangxin gyohuangxin gyohuangxin approved these changes

Assignees
No one assigned
Labels
area/ci Continuous integration | Build and release area/lifecycle Lifecycle management (install, uninstall, configure) related issue/willfix This issue will be worked on
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

CI action error due to inaccessibility of k8s cluster
4 participants
@alphaX86 @hershd23 @gyohuangxin @leecalcote