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 extended resources in providerConfig.nodeTemplate, inherit core resources from pool.nodeTemplate #1010

Merged
merged 6 commits into from
Nov 14, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
8 changes: 5 additions & 3 deletions docs/usage/usage.md
Original file line number Diff line number Diff line change
Expand Up @@ -421,9 +421,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 @@ -201,23 +202,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 @@ -272,7 +271,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 @@ -329,7 +328,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
Loading