gitea/act_runner
61
106
Fork 79
Code Issues 136 Pull Requests 9 Actions Releases 21 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
Copy Link

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
refactor: redesign host mode
All checks were successful
check and test
Details
96ce0b83d7
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
Copy Link

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
Copy Link

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
Copy Link

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
Copy Link

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
Copy Link

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
Copy Link

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
Copy Link

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
Copy Link

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
Copy Link

@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
Copy Link

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/compatible
  
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
confirmed
confirmed issue

  
reviewed
duplicate
This issue or pull request already exists

  
reviewed
invalid
Something is wrong

  
reviewed
needs feedback

  
reviewed
wontfix
This won't be fixed

  
reviewed
workaround

No Label
kind
bug
kind
build
kind/compatible
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
confirmed
reviewed
duplicate
reviewed
invalid
reviewed
needs feedback
reviewed
wontfix
reviewed
workaround
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?