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

Support organization-level runner #97

Merged
merged 11 commits into from
Nov 1, 2021
Merged

Support organization-level runner #97

merged 11 commits into from
Nov 1, 2021

Conversation

kmdkuk
Copy link
Contributor

@kmdkuk kmdkuk commented Oct 12, 2021

#74
Signed-off-by: kouki kouworld0123@gmail.com

@kmdkuk kmdkuk self-assigned this Oct 12, 2021
@kmdkuk kmdkuk changed the title Change the minimum test to an organization-level RunnerPool. WIP Support organization-level runner Oct 12, 2021
@kmdkuk kmdkuk force-pushed the support-org-runner branch 7 times, most recently from 93a3ab1 to cb1f5bb Compare October 15, 2021 09:46
@kmdkuk kmdkuk changed the title WIP Support organization-level runner Support organization-level runner Oct 19, 2021
Signed-off-by: kouki <kouworld0123@gmail.com>
Signed-off-by: kouki <kouworld0123@gmail.com>
@kmdkuk kmdkuk force-pushed the support-org-runner branch 3 times, most recently from e368318 to cc4b31a Compare October 19, 2021 07:46
Signed-off-by: kouki <kouworld0123@gmail.com>
Signed-off-by: kouki <kouworld0123@gmail.com>
@kmdkuk kmdkuk marked this pull request as ready for review October 19, 2021 09:29
constants.go Outdated Show resolved Hide resolved
controllers/runnerpool_controller.go Outdated Show resolved Hide resolved
docs/user-manual.md Outdated Show resolved Hide resolved
docs/user-manual.md Show resolved Hide resolved
Signed-off-by: kouki <kouworld0123@gmail.com>
Signed-off-by: kouki <kouworld0123@gmail.com>
Signed-off-by: kouki <kouworld0123@gmail.com>
@kmdkuk kmdkuk requested a review from masa213f October 27, 2021 08:20
Comment on lines 81 to 82
Short: "set slack channel to notify.",
Long: `This command TODO description`,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Short: "set slack channel to notify.",
Long: `This command TODO description`,
Short: "set slack channel to notify.",
Long: `set slack channel to notify.`,

channel = os.Getenv(constants.SlackChannelEnvName)
}

err = ioutil.WriteFile(slackChannelFilePath, []byte(channel), 0644)
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you read These documents?
https://golang.org/doc/go1.16#ioutil
https://pkg.go.dev/io/ioutil#WriteFile

You can use os.WriteFile instead.

And {os/ioutil}.WriteFile will overwrite if target file exists.
So you need not remove the slack channel file before.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did not know about this information. Thank you.
I'll fix it together with some other places that use ioutil.

if len(args) == 1 {
channel = args[0]
}
err := os.MkdirAll(constants.RunnerVarDirPath, 0755)
Copy link
Contributor

Choose a reason for hiding this comment

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

You should not create /var/meows here.
Since an emptyDir will be mounted in /var/meows, so usually the directory exists.

And if -file /path/to/otherDir/otherFile flag is set, the slack channel will be written in the other dir.

Suggested change
err := os.MkdirAll(constants.RunnerVarDirPath, 0755)
err := os.MkdirAll(constants.RunnerVarDirPath, 0755)

Signed-off-by: kouki <kouworld0123@gmail.com>
Signed-off-by: kouki <kouworld0123@gmail.com>
@kmdkuk
Copy link
Contributor Author

kmdkuk commented Oct 28, 2021

When I commanded echo "#test1" > /var/meows/slack_channel, I forgot to push the updated documentation, so I added cf57f87.

docs/commands.md Outdated
Comment on lines 89 to 91
This sub command set a Slack channel to notified job result.
This is used by calling it in the workflow yaml file.
If it is not specified, the environment variable `MEOWS_SLACK_CHANNEL` is specified.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
This sub command set a Slack channel to notified job result.
This is used by calling it in the workflow yaml file.
If it is not specified, the environment variable `MEOWS_SLACK_CHANNEL` is specified.
This sub command sets a Slack channel that a job result will be notified to.
This command should be called in a workflow.
Users can specify the Slack channel as an argument.
If the argument is not specified, the environment variable `MEOWS_SLACK_CHANNEL` is read instead.


The following methods exist for specifying the channel for Slack notifications.
The priority order of the specification method is 4>3>2>1.
Any channel specification method should look like `#<channel_name>`. (e.g. `#general`, `#test1`)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Any channel specification method should look like `#<channel_name>`. (e.g. `#general`, `#test1`)
Any method accepts a channel name in the format of `#<channel_name>`. (e.g. `#general`, `#test1`)

Comment on lines 200 to 220
1. Slack app secret to be created in [How to deploy Slack agent](#how-to-deploy-slack-agent)
2. `RunnerPool` resource in `.spec.slackAgent.channel`. [docs to SlackAgentConfig](crd-runner-pool.md#SlackAgentConfig)
3. Environment variables in the workflow.
- The `MEOWS_SLACK_CHANNEL` environment variable is read when `job-started` is executed.
4. Call command `meows slackagent set-channel "#channel"` in the workflow step.
- You can specify the channel to be notified by using the command as follows.
```yaml
name: slack-channel-specified
on: push

jobs:
build:
name: job-name
env:
MEOWS_SLACK_CHANNEL: "#test2"
steps:
- run: job-started
# The environment variable `MEOWS_SLACK_CHANNEL` is ignored and the job results are reported to the "#test1" channel.
- run: meows slackagent set-channel "#test1"
- run: job-success
```
Copy link
Contributor

Choose a reason for hiding this comment

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

1. `SLACK_CHANNEL` value in the `slack-app-secret` secret. See [How to deploy Slack agent](#how-to-deploy-slack-agent).
2. `.spec.slackAgent.channel` field in a `RunnerPool` resource. See [SlackAgentConfig](crd-runner-pool.md#SlackAgentConfig).
3. `MEOWS_SLACK_CHANNEL` environment variable in a workflow.
4. Call `meows slackagent set-channel "#channel"` command in a workflow.

For example, you can specify the channel in a workflow as follows.

```yaml
name: slack-channel-specified
on: push

jobs:
  build:
    name: job-name
    env:
      # Basically, a job result will be reported to the "#test1" channel.
      MEOWS_SLACK_CHANNEL: "#test1"
    steps:
      - run: job-started
      # Only when a job fails, the result will be reported to the "#test2" channel.
      - run: meows slackagent set-channel "#test2"
        if: failure()
      - run: job-success
```

Comment on lines 81 to 83
Long: `This command set a Slack channel to notified job result.
This is used by calling it in the workflow yaml file.
If SLACK_CHANNEL_NAME is not specified, the environment variable MEOWS_SLACK_CHANNEL is specified as a Slack channel to notified.`,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Long: `This command set a Slack channel to notified job result.
This is used by calling it in the workflow yaml file.
If SLACK_CHANNEL_NAME is not specified, the environment variable MEOWS_SLACK_CHANNEL is specified as a Slack channel to notified.`,
Long: `This command sets a Slack channel that a job result to be notified to.
This should be called in a workflow.
If SLACK_CHANNEL_NAME is not specified, the environment variable MEOWS_SLACK_CHANNEL is read.`,

@@ -25,6 +25,8 @@ var reservedEnvNames = map[string]bool{
// RunnerPoolSpec defines the desired state of RunnerPool
type RunnerPoolSpec struct {
// RepositoryName describes repository name to register Pods as self-hosted runners.
// RepositoryName is not necessary, If you want to use it as an organization-level self-hosted runner
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// RepositoryName is not necessary, If you want to use it as an organization-level self-hosted runner
// If this field is omitted or the empty string (`""`) is specified, Pods will be registered as organization-level self-hosted runners.

Signed-off-by: kouki <kouworld0123@gmail.com>
Copy link
Contributor

@masa213f masa213f left a comment

Choose a reason for hiding this comment

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

LGTM

@kmdkuk kmdkuk merged commit 24bfa2d into main Nov 1, 2021
@kmdkuk kmdkuk deleted the support-org-runner branch November 1, 2021 01:14
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.

2 participants