Skip to content

Commit

Permalink
support extended resources in providerConfig.nodeTemplate, inherit co…
Browse files Browse the repository at this point in the history
…re resources from pool.nodeTemplate (#1010)

* support extended resources without re-specifying core resources

* added unit test for extended resources

* updated usage doc for extended resource
  • Loading branch information
elankath authored Nov 14, 2024
1 parent 1e287fc commit 9b57681
Show file tree
Hide file tree
Showing 9 changed files with 89 additions and 45 deletions.
8 changes: 5 additions & 3 deletions docs/usage/usage.md
Original file line number Diff line number Diff line change
Expand Up @@ -423,9 +423,11 @@ spec:
# arn: my-instance-profile-arn
nodeTemplate: # (to be specified only if the node capacity would be different from cloudprofile info during runtime)
capacity:
cpu: 2
gpu: 0
memory: 50Gi
cpu: 2 # inherited from pool's machine type if un-specified
gpu: 0 # inherited from pool's machine type if un-specified
memory: 50Gi # inherited from pool's machine type if un-specified
ephemeral-storage: 10Gi # override to specify explicit ephemeral-storage for scale fro zero
resource.com/dongle: 4 # Example of a custom, extended resource.
```

The `.volume.iops` is the number of I/O operations per second (IOPS) that the volume supports.
Expand Down
6 changes: 5 additions & 1 deletion pkg/apis/aws/validation/worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,14 @@ func ValidateWorkerConfig(workerConfig *apisaws.WorkerConfig, volume *core.Volum
}

if nodeTemplate := workerConfig.NodeTemplate; nodeTemplate != nil {
if len(nodeTemplate.Capacity) == 0 {
allErrs = append(allErrs, field.Required(fldPath.Child("nodeTemplate").Child("capacity"), "capacity must not be empty"))
}
for _, capacityAttribute := range []corev1.ResourceName{"cpu", "gpu", "memory"} {
value, ok := nodeTemplate.Capacity[capacityAttribute]
if !ok {
allErrs = append(allErrs, field.Required(fldPath.Child("nodeTemplate").Child("capacity"), fmt.Sprintf("%s is a mandatory field", capacityAttribute)))
// core resources such as "cpu", "gpu", "memory" need not all be explicitly specified in workerConfig.NodeTemplate.
// Will fall back to the worker pool's node template if missing.
continue
}
allErrs = append(allErrs, validateResourceQuantityValue(capacityAttribute, value, fldPath.Child("nodeTemplate").Child("capacity").Child(string(capacityAttribute)))...)
Expand Down
9 changes: 3 additions & 6 deletions pkg/apis/aws/validation/worker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,19 +113,16 @@ var _ = Describe("ValidateWorkerConfig", func() {
}))))
})

It("should return errors for a invalid nodetemplate configuration", func() {
It("should return error for an empty nodetemplate capacity", func() {
worker.NodeTemplate = &extensionsv1alpha1.NodeTemplate{
Capacity: corev1.ResourceList{
"memory": resource.MustParse("50Gi"),
"gpu": resource.MustParse("0"),
},
Capacity: corev1.ResourceList{},
}
errorList := ValidateWorkerConfig(worker, rootVolumeIO1, dataVolumes, fldPath)

Expect(errorList).To(ConsistOf(PointTo(MatchFields(IgnoreExtras, Fields{
"Type": Equal(field.ErrorTypeRequired),
"Field": Equal("config.nodeTemplate.capacity"),
"Detail": Equal("cpu is a mandatory field"),
"Detail": Equal("capacity must not be empty"),
}))))
})

Expand Down
10 changes: 8 additions & 2 deletions pkg/controller/worker/actuator.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func (d *delegateFactory) WorkerDelegate(_ context.Context, worker *extensionsv1
)
}

type workerDelegate struct {
type WorkerDelegate struct {
client client.Client
decoder runtime.Decoder
scheme *runtime.Scheme
Expand Down Expand Up @@ -116,7 +116,7 @@ func NewWorkerDelegate(
if err != nil {
return nil, err
}
return &workerDelegate{
return &WorkerDelegate{
client: client,
decoder: decoder,
scheme: scheme,
Expand All @@ -129,3 +129,9 @@ func NewWorkerDelegate(
worker: worker,
}, nil
}

// GetMachineClasses returns the islice of machine classes contained inside the worker delegate.
// Introduced for Unit-testing.
func (w *WorkerDelegate) GetMachineClasses() []map[string]any {
return w.machineClasses
}
4 changes: 2 additions & 2 deletions pkg/controller/worker/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (
"github.com/gardener/gardener-extension-provider-aws/pkg/apis/aws/v1alpha1"
)

func (w *workerDelegate) decodeWorkerProviderStatus() (*api.WorkerStatus, error) {
func (w *WorkerDelegate) decodeWorkerProviderStatus() (*api.WorkerStatus, error) {
workerStatus := &api.WorkerStatus{}

if w.worker.Status.ProviderStatus == nil {
Expand All @@ -30,7 +30,7 @@ func (w *workerDelegate) decodeWorkerProviderStatus() (*api.WorkerStatus, error)
return workerStatus, nil
}

func (w *workerDelegate) updateWorkerProviderStatus(ctx context.Context, workerStatus *api.WorkerStatus) error {
func (w *WorkerDelegate) updateWorkerProviderStatus(ctx context.Context, workerStatus *api.WorkerStatus) error {
var workerStatusV1alpha1 = &v1alpha1.WorkerStatus{
TypeMeta: metav1.TypeMeta{
APIVersion: v1alpha1.SchemeGroupVersion.String(),
Expand Down
12 changes: 6 additions & 6 deletions pkg/controller/worker/machine_dependencies.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,31 +9,31 @@ import (
)

// DeployMachineDependencies implements genericactuator.WorkerDelegate.
func (w *workerDelegate) DeployMachineDependencies(_ context.Context) error {
func (w *WorkerDelegate) DeployMachineDependencies(_ context.Context) error {
return nil
}

// CleanupMachineDependencies implements genericactuator.WorkerDelegate.
func (w *workerDelegate) CleanupMachineDependencies(_ context.Context) error {
func (w *WorkerDelegate) CleanupMachineDependencies(_ context.Context) error {
return nil
}

// PreReconcileHook implements genericactuator.WorkerDelegate.
func (w *workerDelegate) PreReconcileHook(_ context.Context) error {
func (w *WorkerDelegate) PreReconcileHook(_ context.Context) error {
return nil
}

// PostReconcileHook implements genericactuator.WorkerDelegate.
func (w *workerDelegate) PostReconcileHook(_ context.Context) error {
func (w *WorkerDelegate) PostReconcileHook(_ context.Context) error {
return nil
}

// PreDeleteHook implements genericactuator.WorkerDelegate.
func (w *workerDelegate) PreDeleteHook(_ context.Context) error {
func (w *WorkerDelegate) PreDeleteHook(_ context.Context) error {
return nil
}

// PostDeleteHook implements genericactuator.WorkerDelegate.
func (w *workerDelegate) PostDeleteHook(_ context.Context) error {
func (w *WorkerDelegate) PostDeleteHook(_ context.Context) error {
return nil
}
4 changes: 2 additions & 2 deletions pkg/controller/worker/machine_images.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (
)

// UpdateMachineImagesStatus implements genericactuator.WorkerDelegate.
func (w *workerDelegate) UpdateMachineImagesStatus(ctx context.Context) error {
func (w *WorkerDelegate) UpdateMachineImagesStatus(ctx context.Context) error {
if w.machineImages == nil {
if err := w.generateMachineConfig(ctx); err != nil {
return fmt.Errorf("unable to generate the machine config: %w", err)
Expand All @@ -37,7 +37,7 @@ func (w *workerDelegate) UpdateMachineImagesStatus(ctx context.Context) error {
return nil
}

func (w *workerDelegate) findMachineImage(name, version string, region string, arch *string) (string, error) {
func (w *WorkerDelegate) findMachineImage(name, version string, region string, arch *string) (string, error) {
ami, err := helper.FindAMIForRegionFromCloudProfile(w.cloudProfileConfig, name, version, region, arch)
if err == nil {
return ami, nil
Expand Down
35 changes: 17 additions & 18 deletions pkg/controller/worker/machines.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package worker
import (
"context"
"fmt"
"maps"
"path/filepath"
"slices"
"sort"
Expand Down Expand Up @@ -54,22 +55,22 @@ func init() {
}

// MachineClassKind yields the name of the machine class kind used by AWS provider.
func (w *workerDelegate) MachineClassKind() string {
func (w *WorkerDelegate) MachineClassKind() string {
return "MachineClass"
}

// MachineClassList yields a newly initialized MachineClassList object.
func (w *workerDelegate) MachineClassList() client.ObjectList {
func (w *WorkerDelegate) MachineClassList() client.ObjectList {
return &machinev1alpha1.MachineClassList{}
}

// MachineClass yields a newly initialized MachineClass object.
func (w *workerDelegate) MachineClass() client.Object {
func (w *WorkerDelegate) MachineClass() client.Object {
return &machinev1alpha1.MachineClass{}
}

// DeployMachineClasses generates and creates the AWS specific machine classes.
func (w *workerDelegate) DeployMachineClasses(ctx context.Context) error {
func (w *WorkerDelegate) DeployMachineClasses(ctx context.Context) error {
if w.machineClasses == nil {
if err := w.generateMachineConfig(ctx); err != nil {
return err
Expand All @@ -80,7 +81,7 @@ func (w *workerDelegate) DeployMachineClasses(ctx context.Context) error {
}

// GenerateMachineDeployments generates the configuration for the desired machine deployments.
func (w *workerDelegate) GenerateMachineDeployments(ctx context.Context) (worker.MachineDeployments, error) {
func (w *WorkerDelegate) GenerateMachineDeployments(ctx context.Context) (worker.MachineDeployments, error) {
if w.machineDeployments == nil {
if err := w.generateMachineConfig(ctx); err != nil {
return nil, err
Expand All @@ -89,7 +90,7 @@ func (w *workerDelegate) GenerateMachineDeployments(ctx context.Context) (worker
return w.machineDeployments, nil
}

func (w *workerDelegate) generateMachineConfig(ctx context.Context) error {
func (w *WorkerDelegate) generateMachineConfig(ctx context.Context) error {
var (
machineDeployments = worker.MachineDeployments{}
machineClasses []map[string]interface{}
Expand Down Expand Up @@ -200,23 +201,21 @@ func (w *workerDelegate) generateMachineConfig(ctx context.Context) error {
machineClassSpec["keyName"] = infrastructureStatus.EC2.KeyName
}

if workerConfig.NodeTemplate != nil {
machineClassSpec["nodeTemplate"] = machinev1alpha1.NodeTemplate{
Capacity: workerConfig.NodeTemplate.Capacity,
InstanceType: pool.MachineType,
Region: w.worker.Spec.Region,
Zone: zone,
Architecture: &arch,
}
} else if pool.NodeTemplate != nil {
machineClassSpec["nodeTemplate"] = machinev1alpha1.NodeTemplate{
var nodeTemplate machinev1alpha1.NodeTemplate
if pool.NodeTemplate != nil {
nodeTemplate = machinev1alpha1.NodeTemplate{
Capacity: pool.NodeTemplate.Capacity,
InstanceType: pool.MachineType,
Region: w.worker.Spec.Region,
Zone: zone,
Architecture: &arch,
}
}
if workerConfig.NodeTemplate != nil {
// Support providerConfig extended resources by copying into node template capacity
maps.Copy(nodeTemplate.Capacity, workerConfig.NodeTemplate.Capacity)
}
machineClassSpec["nodeTemplate"] = nodeTemplate

if cpuOptions := workerConfig.CpuOptions; cpuOptions != nil {
machineClassSpec["cpuOptions"] = map[string]int64{
Expand Down Expand Up @@ -271,7 +270,7 @@ func (w *workerDelegate) generateMachineConfig(ctx context.Context) error {
return nil
}

func (w *workerDelegate) computeBlockDevices(pool extensionsv1alpha1.WorkerPool, workerConfig *awsapi.WorkerConfig) ([]map[string]interface{}, error) {
func (w *WorkerDelegate) computeBlockDevices(pool extensionsv1alpha1.WorkerPool, workerConfig *awsapi.WorkerConfig) ([]map[string]interface{}, error) {
var blockDevices []map[string]interface{}

// handle root disk
Expand Down Expand Up @@ -328,7 +327,7 @@ func (w *workerDelegate) computeBlockDevices(pool extensionsv1alpha1.WorkerPool,
return blockDevices, nil
}

func (w *workerDelegate) generateWorkerPoolHash(pool extensionsv1alpha1.WorkerPool, workerConfig awsapi.WorkerConfig) (string, error) {
func (w *WorkerDelegate) generateWorkerPoolHash(pool extensionsv1alpha1.WorkerPool, workerConfig awsapi.WorkerConfig) (string, error) {
return worker.WorkerPoolHash(pool, w.cluster, computeAdditionalHashDataV1(pool), computeAdditionalHashDataV2(pool, workerConfig))

}
Expand Down
46 changes: 41 additions & 5 deletions pkg/controller/worker/machines_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"context"
"encoding/json"
"fmt"
"maps"
"path/filepath"
"strconv"
"strings"
Expand Down Expand Up @@ -63,7 +64,7 @@ var _ = Describe("Machines", func() {
ctrl.Finish()
})

Context("workerDelegate", func() {
Context("WorkerDelegate", func() {
workerDelegate, _ := NewWorkerDelegate(nil, nil, nil, nil, "", nil, nil)

Describe("#GenerateMachineDeployments, #DeployMachineClasses", func() {
Expand Down Expand Up @@ -699,7 +700,7 @@ var _ = Describe("Machines", func() {

expectedUserDataSecretRefRead()

// Test workerDelegate.DeployMachineClasses()
// Test WorkerDelegate.DeployMachineClasses()
chartApplier.EXPECT().ApplyFromEmbeddedFS(
ctx,
charts.InternalChart,
Expand All @@ -712,7 +713,7 @@ var _ = Describe("Machines", func() {
err := workerDelegate.DeployMachineClasses(ctx)
Expect(err).NotTo(HaveOccurred())

// Test workerDelegate.UpdateMachineDeployments()
// Test WorkerDelegate.UpdateMachineDeployments()
expectedImages := &apiv1alpha1.WorkerStatus{
TypeMeta: metav1.TypeMeta{
APIVersion: apiv1alpha1.SchemeGroupVersion.String(),
Expand Down Expand Up @@ -745,7 +746,7 @@ var _ = Describe("Machines", func() {
err = workerDelegate.UpdateMachineImagesStatus(ctx)
Expect(err).NotTo(HaveOccurred())

// Test workerDelegate.GenerateMachineDeployments()
// Test WorkerDelegate.GenerateMachineDeployments()

result, err := workerDelegate.GenerateMachineDeployments(ctx)
Expect(err).NotTo(HaveOccurred())
Expand All @@ -765,7 +766,7 @@ var _ = Describe("Machines", func() {

expectedUserDataSecretRefRead()

// Test workerDelegate.DeployMachineClasses()
// Test WorkerDelegate.DeployMachineClasses()
chartApplier.EXPECT().ApplyFromEmbeddedFS(
ctx,
charts.InternalChart,
Expand Down Expand Up @@ -855,6 +856,41 @@ var _ = Describe("Machines", func() {
err := workerDelegate.DeployMachineClasses(context.TODO())
Expect(err).To(HaveOccurred())
})

It("should return generate machine classes with core and extended resources in the nodeTemplate", func() {
ephemeralStorageQuant := resource.MustParse("30Gi")
dongleName := corev1.ResourceName("resources.com/dongle")
dongleQuant := resource.MustParse("4")
customResources := corev1.ResourceList{
corev1.ResourceEphemeralStorage: ephemeralStorageQuant,
dongleName: dongleQuant,
}
w.Spec.Pools[0].ProviderConfig = &runtime.RawExtension{
Raw: encode(&api.WorkerConfig{
NodeTemplate: &extensionsv1alpha1.NodeTemplate{
Capacity: customResources,
},
}),
}

expectedCapacity := w.Spec.Pools[0].NodeTemplate.Capacity.DeepCopy()
maps.Copy(expectedCapacity, customResources)

wd, err := NewWorkerDelegate(c, decoder, scheme, chartApplier, "", w, cluster)
Expect(err).NotTo(HaveOccurred())
expectedUserDataSecretRefRead()
_, err = wd.GenerateMachineDeployments(ctx)
Expect(err).NotTo(HaveOccurred())
workerDelegate := wd.(*WorkerDelegate)
mClasses := workerDelegate.GetMachineClasses()
for _, mClz := range mClasses {
className := mClz["name"].(string)
if strings.Contains(className, namePool1) {
nt := mClz["nodeTemplate"].(machinev1alpha1.NodeTemplate)
Expect(nt.Capacity).To(Equal(expectedCapacity))
}
}
})
})

It("should fail because the version is invalid", func() {
Expand Down

0 comments on commit 9b57681

Please sign in to comment.