Skip to content

Commit

Permalink
fix(cloudprovider/externalgrpc): GetOptions returns `MaxNodeProvisi…
Browse files Browse the repository at this point in the history
…onTime` as 0 seconds

Signed-off-by: vadasambar <[email protected]>

chore(cloudprovider/externalgrpc): fix CI failing because of wrong header
- CI complains if I remove or change the copyright header
Signed-off-by: vadasambar <[email protected]>

chore(cloudprovider/externalgrpc): fix spacing for license headers
Signed-off-by: vadasambar <[email protected]>

chore(cloudprovider/externalgrpc): remove extra space in copyright header
Signed-off-by: vadasambar <[email protected]>

test(cloudprovider/externalgrpc): add assert for `MaxNodeProvisionTime`
- add extra assert for err test case
Signed-off-by: vadasambar <[email protected]>

fix: duplicate line
Signed-off-by: vadasambar <[email protected]>

docs: add example with `MaxNodeProvisionTime`
Signed-off-by: vadasambar <[email protected]>

refactor: return `MaxNodeProvisionTime` from `NodeGroupGetOptions` in response
Signed-off-by: vadasambar <[email protected]>
  • Loading branch information
vadasambar committed Jul 13, 2023
1 parent b569db4 commit 6df52ec
Show file tree
Hide file tree
Showing 7 changed files with 247 additions and 212 deletions.
4 changes: 2 additions & 2 deletions cluster-autoscaler/cloudprovider/externalgrpc/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ To regenerate the gRPC code:
1. install `protoc` and `protoc-gen-go-grpc`:

```bash
go install google.golang.org/protobuf/cmd/protoc-gen-go@v1.26
go install google.golang.org/grpc/cmd/protoc-gen-go-grpc@v1.1
go install google.golang.org/protobuf/cmd/protoc-gen-go@v1.28
go install google.golang.org/grpc/cmd/protoc-gen-go-grpc@v1.2
```

2. generate gRPC client and server code:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,7 @@ func (w *Wrapper) NodeGroupGetOptions(_ context.Context, req *protos.NodeGroupAu
ScaleDownGpuUtilizationThreshold: pbDefaults.GetScaleDownGpuUtilizationThreshold(),
ScaleDownUnneededTime: pbDefaults.GetScaleDownUnneededTime().Duration,
ScaleDownUnreadyTime: pbDefaults.GetScaleDownUnneededTime().Duration,
MaxNodeProvisionTime: pbDefaults.GetMaxNodeProvisionTime().Duration,
}
opts, err := ng.GetOptions(defaults)
if err != nil {
Expand All @@ -369,6 +370,9 @@ func (w *Wrapper) NodeGroupGetOptions(_ context.Context, req *protos.NodeGroupAu
ScaleDownUnreadyTime: &metav1.Duration{
Duration: opts.ScaleDownUnreadyTime,
},
MaxNodeProvisionTime: &metav1.Duration{
Duration: opts.MaxNodeProvisionTime,
},
},
}, nil
}
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,9 @@ func (n *NodeGroup) GetOptions(defaults config.NodeGroupAutoscalingOptions) (*co
ScaleDownUnreadyTime: &metav1.Duration{
Duration: defaults.ScaleDownUnreadyTime,
},
MaxNodeProvisionTime: &metav1.Duration{
Duration: defaults.MaxNodeProvisionTime,
},
},
})
if err != nil {
Expand All @@ -278,6 +281,7 @@ func (n *NodeGroup) GetOptions(defaults config.NodeGroupAutoscalingOptions) (*co
ScaleDownGpuUtilizationThreshold: pbOpts.GetScaleDownGpuUtilizationThreshold(),
ScaleDownUnneededTime: pbOpts.GetScaleDownUnneededTime().Duration,
ScaleDownUnreadyTime: pbOpts.GetScaleDownUnreadyTime().Duration,
MaxNodeProvisionTime: pbOpts.GetMaxNodeProvisionTime().Duration,
}
return opts, nil
}
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,7 @@ func TestCloudProvider_GetOptions(t *testing.T) {
ScaleDownGpuUtilizationThreshold: 0.7,
ScaleDownUnneededTime: &v1.Duration{Duration: time.Minute},
ScaleDownUnreadyTime: &v1.Duration{Duration: time.Hour},
MaxNodeProvisionTime: &v1.Duration{Duration: time.Minute},
},
},
nil,
Expand All @@ -240,6 +241,7 @@ func TestCloudProvider_GetOptions(t *testing.T) {
ScaleDownGpuUtilizationThreshold: 0.7,
ScaleDownUnneededTime: time.Minute,
ScaleDownUnreadyTime: time.Hour,
MaxNodeProvisionTime: time.Minute,
}

opts, err := ng1.GetOptions(defaultsOpts)
Expand All @@ -248,6 +250,7 @@ func TestCloudProvider_GetOptions(t *testing.T) {
assert.Equal(t, 0.7, opts.ScaleDownGpuUtilizationThreshold)
assert.Equal(t, time.Minute, opts.ScaleDownUnneededTime)
assert.Equal(t, time.Hour, opts.ScaleDownUnreadyTime)
assert.Equal(t, time.Minute, opts.MaxNodeProvisionTime)

// test grpc error
m.On(
Expand All @@ -264,8 +267,9 @@ func TestCloudProvider_GetOptions(t *testing.T) {
client: client,
}

_, err = ng2.GetOptions(defaultsOpts)
opts, err = ng2.GetOptions(defaultsOpts)
assert.Error(t, err)
assert.Nil(t, opts)

// test no opts
m.On(
Expand Down
434 changes: 225 additions & 209 deletions cluster-autoscaler/cloudprovider/externalgrpc/protos/externalgrpc.pb.go

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,9 @@ message NodeGroupAutoscalingOptions {
// ScaleDownUnreadyTime represents how long an unready node should be
// unneeded before it is eligible for scale down.
k8s.io.apimachinery.pkg.apis.meta.v1.Duration scaleDownUnreadyTime = 4;

// MaxNodeProvisionTime time CA waits for node to be provisioned
k8s.io.apimachinery.pkg.apis.meta.v1.Duration MaxNodeProvisionTime = 5;
}

message NodeGroupAutoscalingOptionsRequest {
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 6df52ec

Please sign in to comment.