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

Run CI on self-hosted machine #499

Merged
merged 6 commits into from
Nov 18, 2024
Merged

Run CI on self-hosted machine #499

merged 6 commits into from
Nov 18, 2024

Conversation

kunxian-xia
Copy link
Collaborator

Closes #451.

Copy link
Collaborator

@matthiasgoergens matthiasgoergens left a comment

Choose a reason for hiding this comment

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

Looks like the runners ain't picking this up, yet?

EDIT: Oh, looks like it's running, but failing (in a way that doesn't give use the X on GitHub.)

I suggest we consider running the CI inside a docker container on the self-hosted machine. Then we can put the config for that container inside of the repository. (Instead of manually setting up the self-hosted machine with the config only known outside of the repository.) @kunxian-xia

@matthiasgoergens
Copy link
Collaborator

Looking at the logs, it looks like the integration test actually succeeded, but didn't manage to tell github about it?

@matthiasgoergens
Copy link
Collaborator

Copy link
Collaborator

@matthiasgoergens matthiasgoergens left a comment

Choose a reason for hiding this comment

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

Please add the Dockerfile (or similar) and scripts necessary to change the container we are running on.

Right now, it works by 'magic', but a reader of the repository would have no clue how to eg change to Ubuntu 24 or anything like that.

@kunxian-xia
Copy link
Collaborator Author

kunxian-xia commented Oct 30, 2024

You can find the Dockerfile and scripts here.

@matthiasgoergens
Copy link
Collaborator

matthiasgoergens commented Oct 30, 2024

You can find the Dockerfile and scripts here.

Please add them to the repository. Thanks.

Please add enough context so that someone else can take what's in the repository and upgrade our container, if necessary.

Adding the scripts to a random gist that's not under the scroll-tech organisation and that you can only find if you go through old PR review comments (and not from the repository source) is pretty much useless. There's also not enough context with just these two scripts to enable anyone to actually do anything with them.

We need this in the repository, so that you can go on holiday, or take a sick day, or get a promotion to CTO or even just go to bed and sleep, and still someone else can figure out how everything works and make changes.

scripts/ci/Dockerfile Show resolved Hide resolved
scripts/ci/Dockerfile Outdated Show resolved Hide resolved
Copy link
Collaborator

@matthiasgoergens matthiasgoergens 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 adding the two files.

Please add some context in the README that describes what someone needs to do to get changes to these files to be reflected in the running CI. I assume it's not automatic like with changes to eg lints.yml?

@kunxian-xia
Copy link
Collaborator Author

Please add some context in the README that describes what someone needs to do to get changes to these files to be reflected in the running CI. I assume it's not automatic like with changes to eg lints.yml?

@matthiasgoergens Added in b64ab0f.

@kunxian-xia
Copy link
Collaborator Author

The test scheme::tests::test_rw_lk_expression_combination is hanging for like 30 mins.

Copy link
Collaborator

@matthiasgoergens matthiasgoergens left a comment

Choose a reason for hiding this comment

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

How does doing this manually compare to using the official https://github.com/actions/runner/pkgs/container/actions-runner?

Copy link
Collaborator

@matthiasgoergens matthiasgoergens left a comment

Choose a reason for hiding this comment

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

Please see comments.

@kunxian-xia
Copy link
Collaborator Author

kunxian-xia commented Nov 5, 2024

How does doing this manually compare to using the official https://github.com/actions/runner/pkgs/container/actions-runner?

I don't know how the official container work.

Copy link
Collaborator

@matthiasgoergens matthiasgoergens left a comment

Choose a reason for hiding this comment

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

Let's just use the official image, instead of cobbling together our own. Less hassle that way.

@kunxian-xia
Copy link
Collaborator Author

The default CI runner provided by GitHub is not powerful enough to run the fibonacci example in #368.

@kunxian-xia kunxian-xia dismissed matthiasgoergens’s stale review November 18, 2024 09:20

We can address it in future PR.

@naure
Copy link
Collaborator

naure commented Nov 18, 2024

Have you tried --max-steps=10000 ?

Or any other number of steps. That makes it as fast as you want. You can extrapolate the timings to the full 11_538_921 steps (e.g. multiply by 1153.8921).

@matthiasgoergens
Copy link
Collaborator

Please use the official image, instead of copy and paste.

Copy link
Collaborator

@matthiasgoergens matthiasgoergens left a comment

Choose a reason for hiding this comment

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

Please see my outstanding comments.

Please don't do even more copy and paste, and please provide some comments for how someone would go and fix the runner or make any changes.

(Btw, as far as I can tell our VPN is still insecure. Has the Ceno server been fixed? I can't access it via VPN without endangering my scroll credentials.)

@kunxian-xia
Copy link
Collaborator Author

First of all, I don't know how to use the official github image. If you know how to use that to spin up CI machine, feel free to make a PR.
Secondly, the Dockerfile and start.sh does work for our case, why not just use it?

@matthiasgoergens
Copy link
Collaborator

matthiasgoergens commented Nov 18, 2024

Have you tried learning how to use the official image? Have you hit any blockers I can try to help you with? You are a smart fellow, you can do it!

I would love to set up a self hosted runner and create a new PR (or take over this one), but I can't access the VPN without endangering my username and password. (And you should probably not access the VPN either, until it's either fixed or Sina tells you it's ok.) I reported the problem a while ago, and recently followed up again.

Please have a look at my other comments, too. The PR does not give enough context for someone to fix or upgrade the runner. We already discussed this.

@kunxian-xia kunxian-xia enabled auto-merge (squash) November 18, 2024 14:03
@kunxian-xia kunxian-xia merged commit f003180 into master Nov 18, 2024
6 checks passed
@kunxian-xia kunxian-xia deleted the feat/ci_self_hosted branch November 18, 2024 14:37
@kunxian-xia
Copy link
Collaborator Author

Please refer to https://www.notion.so/scrollzkp/Ceno-s-self-hosted-CI-runner-1437792d22af800eba16e4b971e82619?pvs=4 for more detailed instructions on setting up our private CI runner.

@matthiasgoergens
Copy link
Collaborator

matthiasgoergens commented Nov 19, 2024

Please refer to https://www.notion.so/scrollzkp/Ceno-s-self-hosted-CI-runner-1437792d22af800eba16e4b971e82619?pvs=4 for more detailed instructions on setting up our private CI runner.

Please put that where someone can find it. Put it in the README for example. No one looks in old reviews when their hair is on fire. The instruction on notion aren't particularly useful, either: I still have no clue how to access the server, and it's mostly just copy and pasted what's already in the README here. (I left some comments on notion.)

Having said that, sadly this is better than nothing.. Why do you need to make this so difficult? Are there are any constraints I'm missing? Please educate me, I'm really at a loss.

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.

Run CI on self-hosted machine
3 participants