Skip to content

Commit 61767d8

Browse files
Hide --num-instances from services update help (#500)
Hides the `--num-instances` flag from `render services update` GROW-2662 Support the flag (but hide it), so that if someone does pass it in, we can redirect them to the dashboard instead of fail entirely I backlogged GROW_2661 to implement `render services scale` at Some Point™️ in the future. GitOrigin-RevId: 2b540451cb727e091fb8b2db256512b83e5e22ff
1 parent 296e6fb commit 61767d8

4 files changed

Lines changed: 39 additions & 11 deletions

File tree

cmd/serviceupdate.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,10 @@ Provide configuration updates with flags.`,
9191

9292
// Instance and scaling flags
9393
cmd.Flags().Int("num-instances", 0, "Number of instances")
94+
// num-instances is intentionally rejected for update (see ValidateUpdate).
95+
// Keep the flag registered so we can return a helpful "use the dashboard" error
96+
// instead of Cobra's generic "unknown flag", but hide it from help output.
97+
_ = cmd.Flags().MarkHidden("num-instances")
9498
cmd.Flags().Int("max-shutdown-delay", 0, "Max shutdown delay in seconds")
9599

96100
// Preview and preview generation flags

cmd/serviceupdate_test.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,33 @@ func TestServiceUpdateFlagsRegistration(t *testing.T) {
113113
}
114114
}
115115

116+
// num-instances stays registered so the command can return a helpful
117+
// "use the dashboard" error rather than Cobra's generic "unknown flag", but it
118+
// must be hidden from help output since it is never a valid update.
119+
func TestServiceUpdateNumInstancesFlagIsHidden(t *testing.T) {
120+
cmd := newServiceUpdateTestCmd()
121+
122+
flag := cmd.Flags().Lookup("num-instances")
123+
require.NotNil(t, flag, "num-instances flag should still be registered")
124+
assert.True(t, flag.Hidden, "num-instances flag should be hidden from help")
125+
}
126+
127+
// End-to-end check: render the actual --help output and confirm num-instances
128+
// does not leak into it. --max-shutdown-delay is a sibling visible flag that
129+
// proves the help text rendered, so the absence assertion can't pass vacuously.
130+
func TestServiceUpdateHelpOutputOmitsNumInstances(t *testing.T) {
131+
cmd := newServiceUpdateTestCmd()
132+
var out bytes.Buffer
133+
cmd.SetOut(&out)
134+
cmd.SetArgs([]string{"--help"})
135+
136+
require.NoError(t, cmd.Execute())
137+
138+
help := out.String()
139+
assert.Contains(t, help, "--max-shutdown-delay", "help output should have rendered visible flags")
140+
assert.NotContains(t, help, "--num-instances", "num-instances should be hidden from help output")
141+
}
142+
116143
func TestServiceUpdateCommandStructure(t *testing.T) {
117144
cmd := newServiceUpdateTestCmd()
118145

pkg/types/service/serviceupdate.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,11 @@ func (s ServiceUpdateInput) ValidateUpdate() error {
102102
if !s.hasUpdateFlags() {
103103
return errors.New("at least one update flag must be provided")
104104
}
105+
if s.NumInstances != nil {
106+
// GROW-2261 will add `render services scale`. Once that ships, update this
107+
// message to recommend that command instead of pointing users to the dashboard.
108+
return errors.New("--num-instances is not supported for update (use the dashboard to change instance count)")
109+
}
105110
if s.Previews != nil {
106111
if _, err := ParsePreviewsGeneration(string(*s.Previews)); err != nil {
107112
return err
@@ -173,9 +178,6 @@ func (s ServiceUpdateInput) ValidateForServiceType(serviceType ServiceType) erro
173178
return fmt.Errorf("--%s is not supported for %s", flag, serviceType)
174179
}
175180

176-
if s.NumInstances != nil {
177-
return fmt.Errorf("--num-instances is not supported for update (use the dashboard to change instance count)")
178-
}
179181
if s.HealthCheckPath != nil {
180182
if err := reject("health-check-path", ServiceTypeWebService); err != nil {
181183
return err

pkg/types/service/serviceupdate_test.go

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -109,13 +109,14 @@ func TestValidateUpdate(t *testing.T) {
109109
require.NoError(t, err)
110110
})
111111

112-
t.Run("passes with service ID and num-instances flag", func(t *testing.T) {
112+
t.Run("rejects num-instances before any service-type check", func(t *testing.T) {
113113
svc := servicetypes.ServiceUpdateInput{
114114
NumInstances: pointers.From(3),
115115
ServiceIDOrName: "my-service-id",
116116
}
117117
err := svc.ValidateUpdate()
118-
require.NoError(t, err)
118+
require.Error(t, err)
119+
assert.Contains(t, err.Error(), "--num-instances is not supported for update")
119120
})
120121

121122
t.Run("passes with service ID and maintenance-mode flag", func(t *testing.T) {
@@ -280,12 +281,6 @@ func TestValidateForServiceType(t *testing.T) {
280281
serviceType: servicetypes.ServiceTypePrivateService,
281282
errMsg: "--maintenance-mode is not supported for private_service",
282283
},
283-
{
284-
name: "num-instances on any type",
285-
input: servicetypes.ServiceUpdateInput{NumInstances: pointers.From(3)},
286-
serviceType: servicetypes.ServiceTypeWebService,
287-
errMsg: "--num-instances is not supported for update",
288-
},
289284
{
290285
name: "plan on static site",
291286
input: servicetypes.ServiceUpdateInput{Plan: pointers.From("pro")},

0 commit comments

Comments
 (0)