Skip to content
This repository was archived by the owner on Jan 8, 2024. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changelog/3854.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
```release-note:improvement
cli: Setting configurations in a runner profile no longer resets unspecified configuration
```

```release-note:improvement
cli: A new `-runner-target-any` flag has been added to runner profile set to allow users to specify targeting any runner.
```
87 changes: 57 additions & 30 deletions internal/cli/runner_profile_set.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,14 @@ type RunnerProfileSetCommand struct {
*baseCommand
//TODO(XX): after `-env-vars` as a slice is deprecated, rename flagEnvVar to flagEnvVars
flagName string
flagOCIUrl string
flagOCIUrl *string
flagEnvVar map[string]string
flagEnvVars []string
flagPluginType string
flagPluginType *string
flagPluginConfig string
flagDefault *bool
flagTargetRunnerId string
flagTargetRunnerAny *bool
flagTargetRunnerId *string
flagTargetRunnerLabels map[string]string
}

Expand Down Expand Up @@ -104,14 +105,18 @@ func (c *RunnerProfileSetCommand) Run(args []string) int {
}

// Set target runner for profile
if c.flagTargetRunnerId != "" {
if c.flagTargetRunnerId != nil {
od.TargetRunner = &pb.Ref_Runner{
Target: &pb.Ref_Runner_Id{
Id: &pb.Ref_RunnerId{
Id: c.flagTargetRunnerId,
Id: *c.flagTargetRunnerId,
},
},
}
if c.flagTargetRunnerAny != nil {
c.ui.Output("Both -target-runner-id and -target-runner-any detected, only one can be set at a time. ID takes priority.",
terminal.WithWarningStyle())
}
if c.flagTargetRunnerLabels != nil {
c.ui.Output("Both -target-runner-id and -target-runner-label detected, only one can be set at a time. ID takes priority.",
terminal.WithWarningStyle())
Expand All @@ -124,7 +129,11 @@ func (c *RunnerProfileSetCommand) Run(args []string) int {
},
},
}
} else {
if c.flagTargetRunnerAny != nil {
c.ui.Output("Both -target-runner-id and -target-runner-any detected, only one can be set at a time. ID takes priority.",
terminal.WithWarningStyle())
}
} else if od.TargetRunner == nil || (c.flagTargetRunnerAny != nil && *c.flagTargetRunnerAny) {
od.TargetRunner = &pb.Ref_Runner{Target: &pb.Ref_Runner_Any{}}
}

Expand Down Expand Up @@ -189,16 +198,21 @@ func (c *RunnerProfileSetCommand) Run(args []string) int {
}
}

od.OciUrl = c.flagOCIUrl
od.EnvironmentVariables = map[string]string{}
if od.OciUrl == "" {
od.OciUrl = installutil.DefaultODRImage
}
if c.flagOCIUrl != nil {
od.OciUrl = *c.flagOCIUrl
}

if c.flagDefault != nil {
od.Default = *c.flagDefault
}

if c.flagEnvVars != nil {
//TODO(XX): Deprecate -env-vars and this logic
c.ui.Output(
"Flag '-env-vars' is deprecated, please use flag `-env-var=k=v`",
"Flag '-env-vars' is deprecated, please use flag '-env-var=k=v'",
terminal.WithWarningStyle(),
)
for _, kv := range c.flagEnvVars {
Expand All @@ -207,20 +221,29 @@ func (c *RunnerProfileSetCommand) Run(args []string) int {
od.EnvironmentVariables[kv[:idx]] = kv[idx+1:]
}
}
} else {
od.EnvironmentVariables = c.flagEnvVar
}

if c.flagPluginType == "" {
c.ui.Output(
"Flag '-plugin-type' must be set to a valid plugin type like 'docker' or 'kubernetes'.\n\n%s",
c.Help(),
terminal.WithErrorStyle(),
)
return 1
if c.flagEnvVar != nil {
for k, v := range c.flagEnvVar {
if v == "" {
delete(od.EnvironmentVariables, k)
} else {
od.EnvironmentVariables[k] = v
}
}
}

od.PluginType = c.flagPluginType
if c.flagPluginType != nil || od.PluginType == "" {
if *c.flagPluginType == "" {
c.ui.Output(
"Flag '-plugin-type' must be set to a valid plugin type like 'docker' or 'kubernetes'.\n\n%s",
c.Help(),
terminal.WithErrorStyle(),
)
return 1
}
od.PluginType = *c.flagPluginType
}

// Upsert
_, err := c.project.Client().UpsertOnDemandRunnerConfig(ctx, &pb.UpsertOnDemandRunnerConfigRequest{
Expand Down Expand Up @@ -255,7 +278,7 @@ func (c *RunnerProfileSetCommand) Flags() *flag.Sets {
Usage: "The name of an existing runner profile to update.",
})

f.StringVar(&flag.StringVar{
f.StringPtrVar(&flag.StringPtrVar{
Name: "oci-url",
Target: &c.flagOCIUrl,
Default: installutil.DefaultODRImage,
Expand All @@ -276,11 +299,10 @@ func (c *RunnerProfileSetCommand) Flags() *flag.Sets {
Usage: "DEPRECATED. Please see `-env-var`.",
})

f.StringVar(&flag.StringVar{
Name: "plugin-type",
Target: &c.flagPluginType,
Default: "",
Usage: "The type of the plugin to launch for the on-demand runner, such as aws-ecs, kubernetes, etc.",
f.StringPtrVar(&flag.StringPtrVar{
Name: "plugin-type",
Target: &c.flagPluginType,
Usage: "The type of the plugin to launch for the on-demand runner, such as aws-ecs, kubernetes, etc.",
})

f.StringVar(&flag.StringVar{
Expand All @@ -299,11 +321,10 @@ func (c *RunnerProfileSetCommand) Flags() *flag.Sets {
"otherwise specify its own remote runner.",
})

f.StringVar(&flag.StringVar{
Name: "target-runner-id",
Target: &c.flagTargetRunnerId,
Default: "",
Usage: "ID of the runner to target for this remote runner profile.",
f.StringPtrVar(&flag.StringPtrVar{
Name: "target-runner-id",
Target: &c.flagTargetRunnerId,
Usage: "ID of the runner to target for this remote runner profile.",
})

f.StringMapVar(&flag.StringMapVar{
Expand All @@ -312,6 +333,12 @@ func (c *RunnerProfileSetCommand) Flags() *flag.Sets {
Usage: "Labels on the runner to target for this remote runner profile. " +
"e.g. `-target-runner-label=k=v`. Can be specified multiple times.",
})

f.BoolPtrVar(&flag.BoolPtrVar{
Name: "target-runner-any",
Target: &c.flagTargetRunnerAny,
Usage: "Set profile to target any available runner.",
})
})
}

Expand Down
65 changes: 65 additions & 0 deletions internal/pkg/flag/flag_string_ptr.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
package flag

import (
"github.com/posener/complete"
)

// -- StringPtrVar and stringValue
type StringPtrVar struct {
Name string
Aliases []string
Usage string
Default string
Hidden bool
EnvVar string
Target **string
Completion complete.Predictor
SetHook func(val string)
}

func (f *Set) StringPtrVar(i *StringPtrVar) {
f.VarFlag(&VarFlag{
Name: i.Name,
Aliases: i.Aliases,
Default: i.Default,
Usage: i.Usage,
EnvVar: i.EnvVar,
Value: newStringPtr(i, i.Target, i.Hidden),
Completion: i.Completion,
})
}

type stringPtrValue struct {
v *StringPtrVar
hidden bool
target **string
}

func newStringPtr(v *StringPtrVar, target **string, hidden bool) *stringPtrValue {
return &stringPtrValue{
v: v,
hidden: hidden,
target: target,
}
}

func (s *stringPtrValue) Set(val string) error {
*s.target = &val

if s.v.SetHook != nil {
s.v.SetHook(val)
}

return nil
}

func (s *stringPtrValue) String() string {
if *s.target != nil {
return **s.target
}
return s.v.Default
}

func (s *stringPtrValue) Get() interface{} { return s.target }
func (s *stringPtrValue) Example() string { return "string" }
func (s *stringPtrValue) Hidden() bool { return s.hidden }
75 changes: 75 additions & 0 deletions internal/pkg/flag/flag_string_ptr_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
package flag

import (
"testing"

"github.com/stretchr/testify/assert"
)

func TestStringPtrVar(t *testing.T) {
cases := map[string]struct {
input *string
expected *string
omitValue bool // e.g. -poll instead of -poll=true
shouldErr bool
}{
"omitted": {
expected: nil,
},
// specifying -flag without a value should fail
"no value set": {
omitValue: true,
shouldErr: true,
},
"value passed in": {
input: strPtr("blammo"),
expected: strPtr("blammo"),
},
// empty is equivilant to a user supplying -flag="", and not a scenario
// where the flag is simply omittied.
"empty string as input": {
input: strPtr(""),
expected: strPtr(""),
},
}

for name, c := range cases {
t.Run(name, func(t *testing.T) {
var valA *string
var valB string
sets := NewSets()
{
set := sets.NewSet("A")
set.StringPtrVar(&StringPtrVar{
Name: "a",
Target: &valA,
})
}
{
// borrowed from string_slice_test, just to have another input
set := sets.NewSet("B")
set.StringVar(&StringVar{
Name: "b",
Target: &valB,
})
}

var err error
inputs := []string{"-b=somevalueB"}
if c.input != nil {
inputs = append(inputs, "-a="+*c.input)
}
if c.omitValue == true {
inputs = append(inputs, "-a")
}
err = sets.Parse(inputs)
if c.shouldErr {
assert.Error(t, err)
} else {
assert.NoError(t, err)
}

assert.Equal(t, c.expected, valA)
})
}
}
1 change: 1 addition & 0 deletions website/content/commands/runner-profile-set.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -44,5 +44,6 @@ lifecycle operation.
- `-default` - Indicates that this remote runner profile should be the default for any project that doesn't otherwise specify its own remote runner.
- `-target-runner-id=<string>` - ID of the runner to target for this remote runner profile.
- `-target-runner-label=<key=value>` - Labels on the runner to target for this remote runner profile. e.g. `-target-runner-label=k=v`. Can be specified multiple times.
- `-target-runner-any` - Set profile to target any available runner.
Copy link
Contributor

Choose a reason for hiding this comment

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

very good catch


@include "commands/runner-profile-set_more.mdx"