gitea
/
act_runner
52
67
Fork 53
Code Issues 77 Pull Requests 3 Actions Releases 16 Activity

Clarify labels #69

Merged
wolfogre merged 6 commits from wolfogre/act_runner:feature/host_mode into main 2023-03-23 12:48:33 +00:00
Conversation 4 Commits 6 Files Changed 7 +123 -54
wolfogre commented 2023-03-23 08:59:31 +00:00
Owner

The label will follow the format label[:schema[:args]], and the schema will be host if it's omitted. So

  • ubuntu:docker://node:18: Run jobs with label ubuntu via docker with image node:18
  • ubuntu:host: Run jobs with label ubuntu on the host directly.
  • ubuntu: Same as ubuntu:host.
  • ubuntu:vm:ubuntu-latest: (Just a example, not Implemented) Run jobs with label ubuntu via virtual machine with iso ubuntu-latest.
The label will follow the format `label[:schema[:args]]`, and the schema will be `host` if it's omitted. So - `ubuntu:docker://node:18`: Run jobs with label `ubuntu` via docker with image `node:18` - `ubuntu:host`: Run jobs with label `ubuntu` on the host directly. - `ubuntu`: Same as `ubuntu:host`. - `ubuntu:vm:ubuntu-latest`: (Just a example, not Implemented) Run jobs with label `ubuntu` via virtual machine with iso `ubuntu-latest`.
wolfogre added 1 commit 2023-03-23 08:59:31 +00:00
wolfogre added the
kind
feature
 label 2023-03-23 09:12:08 +00:00
wxiaoguang reviewed 2023-03-23 09:15:45 +00:00
runtime/runtime.go Outdated
@ -45,1 +45,4 @@
if len(splits) != 1 {
continue
}
if len(splits) == 1 {
wxiaoguang commented 2023-03-23 09:15:45 +00:00
Member

There is a if len(splits) != 1 { above?

There is a `if len(splits) != 1 {` above?
wolfogre marked this conversation as resolved
wolfogre added 1 commit 2023-03-23 09:17:37 +00:00
wolfogre reviewed 2023-03-23 09:19:23 +00:00
runtime/runtime.go Outdated
@ -48,3 +45,4 @@
if len(splits) != 2 {
continue
}
// ["ubuntu-18.04", "docker://node:16-buster"]
wolfogre commented 2023-03-23 09:18:56 +00:00
Author
Owner

Typo. 😂

Typo. 😂
wolfogre marked this conversation as resolved
wxiaoguang reviewed 2023-03-23 09:23:37 +00:00
runtime/runtime.go Outdated
@ -57,0 +53,4 @@
// TODO "//" will be ignored, maybe we should use 'ubuntu-18.04:docker:node:16-buster' instead
platforms[k] = strings.TrimPrefix(strings.TrimPrefix(v, "docker:"), "//")
case v == "host":
platforms[k] = "-self-hosted"
wxiaoguang commented 2023-03-23 09:23:37 +00:00
Member

Does it need a default case? At least, show some error logs.

Does it need a `default` case? At least, show some error logs.
👍 1
wolfogre marked this conversation as resolved
Zettat123 approved these changes 2023-03-23 09:24:50 +00:00
runtime/runtime.go Outdated
@ -39,3 +39,3 @@
// "self-hosted"
// "linux_arm:host"
platforms := make(map[string]string, len(labels))
Zettat123 commented 2023-03-23 09:24:35 +00:00
Member

should be make(map[string]string, len(s.Labels)) ?

should be `make(map[string]string, len(s.Labels))` ?
👍 1
wolfogre marked this conversation as resolved
wolfogre added 1 commit 2023-03-23 09:26:58 +00:00
wxiaoguang reviewed 2023-03-23 09:29:51 +00:00
README.md
@ -46,3 +46,3 @@
INFO Enter the runner name (if set empty, use hostname: Test.local):
INFO Enter the runner labels, leave blank to use the default labels (comma-separated, for example, self-hosted,ubuntu-20.04:docker://node:16-bullseye,ubuntu-18.04:docker://node:16-buster):
INFO Enter the runner labels, leave blank to use the default labels (comma-separated, for example, ubuntu-20.04:docker://node:16-bullseye,ubuntu-18.04:docker://node:16-buster,linux_arm:host):
wxiaoguang commented 2023-03-23 09:29:51 +00:00
Member

I guess it could have some more explanations about each part of the "label" .

And it looks strange that linux_arm:host becomes -self-hosted

I guess it could have some more explanations about each part of the "label" . And it looks strange that `linux_arm:host` becomes `-self-hosted`
wolfogre marked this conversation as resolved
ChristopherHX commented 2023-03-23 09:30:16 +00:00
Contributor

Previously, the only way to register a runner for running jobs on a host without containers was to use the label "self-hosted". However, this design was ill-conceived.

This limitation doesn't exists, only documentation wise.

linux_arm as labelstring is also host mode.

> Previously, the only way to register a runner for running jobs on a host without containers was to use the label "self-hosted". However, this design was ill-conceived. This limitation doesn't exists, only documentation wise. `linux_arm` as labelstring is also host mode.
wolfogre commented 2023-03-23 09:43:51 +00:00
Author
Owner

Previously, the only way to register a runner for running jobs on a host without containers was to use the label "self-hosted". However, this design was ill-conceived.

This limitation doesn't exists, only documentation wise.

linux_arm as labelstring is also host mode.

I got it.

I noticed this problem when writing docs, since the original design of complete label is label:schema[:args].
So ubuntu:docker://node:18, and maybe ubuntu:vm:xxxx, ubuntu:cri:xxxx in the future.
That's why I think it should be ubuntu:host. The host is schema.

However, @wxiaoguang has a better idea that the schema can be optional and is "-self-hosted" by default, like label[:schema[:args]].

It makes sense, I will close this PR.

I will rewrite this PR.

> > Previously, the only way to register a runner for running jobs on a host without containers was to use the label "self-hosted". However, this design was ill-conceived. > > This limitation doesn't exists, only documentation wise. > > `linux_arm` as labelstring is also host mode. I got it. I noticed this problem when writing docs, since the original design of complete label is `label:schema[:args]`. So `ubuntu:docker://node:18`, and maybe `ubuntu:vm:xxxx`, `ubuntu:cri:xxxx` in the future. That's why I think it should be `ubuntu:host`. The `host` is schema. However, @wxiaoguang has a better idea that the schema can be optional and is "-self-hosted" by default, like `label[:schema[:args]]`. It makes sense, ~~I will close this PR.~~ I will rewrite this PR.
👍 1
wolfogre closed this pull request 2023-03-23 09:44:42 +00:00
wolfogre reopened this pull request 2023-03-23 09:48:43 +00:00
ChristopherHX commented 2023-03-23 09:58:30 +00:00
Contributor

However, @wxiaoguang has a better idea that the schema can be optional and is "-self-hosted" by default, like label[:schema[:args]].

Yes this would avoid a breaking change. I had choosen to not add a new schema, because host mode doesn't need any args. However it is a valid idea to add one.

Maybe https://github.com/nektos/act/pull/1682 will get an own schema like "lxc://".

> However, @wxiaoguang has a better idea that the schema can be optional and is "-self-hosted" by default, like `label[:schema[:args]]`. Yes this would avoid a breaking change. I had choosen to not add a new schema, because host mode doesn't need any args. However it is a valid idea to add one. Maybe https://github.com/nektos/act/pull/1682 will get an own schema like "lxc://".
👍 1
wolfogre added 1 commit 2023-03-23 10:10:55 +00:00
wolfogre changed title from Redesign host mode to Clarify labels 2023-03-23 10:11:51 +00:00
wolfogre commented 2023-03-23 10:19:44 +00:00
Author
Owner

@ChristopherHX I rewrote this PR, could you please take a look at it again.

@ChristopherHX I rewrote this PR, could you please take a look at it again.
wolfogre added 1 commit 2023-03-23 10:21:30 +00:00
wolfogre added 1 commit 2023-03-23 10:39:15 +00:00
ChristopherHX approved these changes 2023-03-23 11:23:21 +00:00
ChristopherHX left a comment
Contributor

Works perfectly fine. I see you have removed the label parsing code I had duplicated in my change.

Works perfectly fine. I see you have removed the label parsing code I had duplicated in my change.
wxiaoguang approved these changes 2023-03-23 11:49:31 +00:00
wolfogre merged commit 90b8cc6a7a into main 2023-03-23 12:48:33 +00:00
wolfogre referenced this issue from a commit 2023-03-23 12:48:34 +00:00
Clarify labels (#69)
lunny approved these changes 2023-03-23 12:49:14 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Zettat123
ChristopherHX
wxiaoguang
lunny
Labels
Clear labels   
kind
bug
Something is not working   
kind
build
   
kind
dependencies
   
kind
docs
   
kind
enhancement
New feature   
kind
feature
   
kind
help wanted
Need some help   
kind
proposal
   
kind
refactor
  
related
act
Related to act.   
related
environment
Related to the environment.   
related
exec
   
related
gitea
Related to Gitea.   
related
workflow
Related to the workflow file or actions scripts.
  
reviewed
duplicate
This issue or pull request already exists   
reviewed
invalid
Something is wrong   
reviewed
needs feedback
   
reviewed
wontfix
This won't be fixed
No Label
kind
bug
kind
build
kind
dependencies
kind
docs
kind
enhancement
kind
feature
kind
help wanted
kind
proposal
kind
refactor
related
act
related
environment
related
exec
related
gitea
related
workflow
reviewed
duplicate
reviewed
invalid
reviewed
needs feedback
reviewed
wontfix
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
5 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: gitea/act_runner#69
No description provided.
Delete Branch "wolfogre/act_runner:feature/host_mode"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?