Skip to content

Commit

Permalink
Remove the ComputeNodes array from SystemConfiguration
Browse files Browse the repository at this point in the history
The list of compute node names is already found within the spec.StorageNodes
array. By having the names in just one array, rather than two, that ought to
help reduce errors.

However, the main problem was this: The spec.ComputeNodes array can account for
up to 75% of the size of the resource on a large cluster.  This has caused
problems for 'kubectl apply' (client-side-apply) when it creates the
"last-applied-configuration" annotation for the resource, with the serialized
version of the resource exceeding the maximum allowed size of an annotation.

Signed-off-by: Dean Roehrich <[email protected]>
  • Loading branch information
roehrich-hpe committed Feb 1, 2024
1 parent 4a08b53 commit 3985b6f
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 90 deletions.
18 changes: 17 additions & 1 deletion api/v1alpha1/conversion.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2023 Hewlett Packard Enterprise Development LP
* Copyright 2023-2024 Hewlett Packard Enterprise Development LP
* Other additional copyright holders may be indicated within.
*
* The entirety of this work is licensed under the Apache License,
Expand Down Expand Up @@ -348,6 +348,10 @@ func (src *SystemConfiguration) ConvertTo(dstRaw conversion.Hub) error {

if hasAnno {
dst.Spec.PortsCooldownInSeconds = restored.Spec.PortsCooldownInSeconds

// dst.Spec.ComputeNodes --The destination does not have this; instead,
// it finds the computes that are already in the dst.Spec.StorageNodes
// list.
}
return nil
}
Expand All @@ -360,6 +364,14 @@ func (dst *SystemConfiguration) ConvertFrom(srcRaw conversion.Hub) error {
return err
}

// The v1alpha1 resource has the compute nodes in both the spec.ComputeNodes
// list and in the spec.StorageNodes list.
computes := make([]SystemConfigurationComputeNode, 0)
for _, name := range src.Computes() {
computes = append(computes, SystemConfigurationComputeNode{Name: name})
}
dst.Spec.ComputeNodes = computes

// Preserve Hub data on down-conversion except for metadata.
return utilconversion.MarshalData(src, dst)
}
Expand Down Expand Up @@ -509,6 +521,10 @@ func Convert_v1alpha2_WorkflowSpec_To_v1alpha1_WorkflowSpec(in *dwsv1alpha2.Work
return autoConvert_v1alpha2_WorkflowSpec_To_v1alpha1_WorkflowSpec(in, out, s)
}

func Convert_v1alpha1_SystemConfigurationSpec_To_v1alpha2_SystemConfigurationSpec(in *SystemConfigurationSpec, out *dwsv1alpha2.SystemConfigurationSpec, s apiconversion.Scope) error {
return autoConvert_v1alpha1_SystemConfigurationSpec_To_v1alpha2_SystemConfigurationSpec(in, out, s)
}

func Convert_v1alpha2_SystemConfigurationSpec_To_v1alpha1_SystemConfigurationSpec(in *dwsv1alpha2.SystemConfigurationSpec, out *SystemConfigurationSpec, s apiconversion.Scope) error {
return autoConvert_v1alpha2_SystemConfigurationSpec_To_v1alpha1_SystemConfigurationSpec(in, out, s)
}
39 changes: 36 additions & 3 deletions api/v1alpha1/conversion_test.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2023 Hewlett Packard Enterprise Development LP
* Copyright 2023-2024 Hewlett Packard Enterprise Development LP
* Other additional copyright holders may be indicated within.
*
* The entirety of this work is licensed under the Apache License,
Expand All @@ -22,7 +22,10 @@ package v1alpha1
import (
"testing"

fuzz "github.com/google/gofuzz"
. "github.com/onsi/ginkgo/v2"
"k8s.io/apimachinery/pkg/api/apitesting/fuzzer"
runtimeserializer "k8s.io/apimachinery/pkg/runtime/serializer"

dwsv1alpha2 "github.com/DataWorkflowServices/dws/api/v1alpha2"
utilconversion "github.com/DataWorkflowServices/dws/github/cluster-api/util/conversion"
Expand Down Expand Up @@ -66,8 +69,9 @@ func TestFuzzyConversion(t *testing.T) {
}))

t.Run("for SystemConfiguration", utilconversion.FuzzTestFunc(utilconversion.FuzzTestFuncInput{
Hub: &dwsv1alpha2.SystemConfiguration{},
Spoke: &SystemConfiguration{},
Hub: &dwsv1alpha2.SystemConfiguration{},
Spoke: &SystemConfiguration{},
FuzzerFuncs: []fuzzer.FuzzerFuncs{SystemConfigurationFuzzFunc},
}))

t.Run("for Workflow", utilconversion.FuzzTestFunc(utilconversion.FuzzTestFuncInput{
Expand All @@ -77,6 +81,35 @@ func TestFuzzyConversion(t *testing.T) {

}

func SystemConfigurationFuzzFunc(_ runtimeserializer.CodecFactory) []interface{} {
return []interface{}{
SystemConfigurationComputesFuzzer,
}
}

// Use the same compute names in both spec.ComputeNodes and spec.StorageNodes.
func SystemConfigurationComputesFuzzer(in *SystemConfigurationSpec, c fuzz.Continue) {
// Tell the fuzzer to begin by fuzzing everything in the object.
c.FuzzNoCustom(in)

// Now pull any fuzzed compute names out of the spec.StorageNodes list and
// use them to build a new spec.ComputeNodes list.
if in.StorageNodes != nil {
computes := make([]SystemConfigurationComputeNode, 0)
for sidx := range in.StorageNodes {
for cidx := range in.StorageNodes[sidx].ComputesAccess {
name := c.RandString()
in.StorageNodes[sidx].ComputesAccess[cidx].Name = name
computes = append(computes, SystemConfigurationComputeNode{Name: name})
}
}
in.ComputeNodes = computes
} else {
in.ComputeNodes = nil
}

}

// Just touch ginkgo, so it's here to interpret any ginkgo args from
// "make test", so that doesn't fail on this test file.
var _ = BeforeSuite(func() {})
50 changes: 7 additions & 43 deletions api/v1alpha1/zz_generated.conversion.go

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

21 changes: 11 additions & 10 deletions api/v1alpha2/systemconfiguration_types.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2021-2023 Hewlett Packard Enterprise Development LP
* Copyright 2021-2024 Hewlett Packard Enterprise Development LP
* Other additional copyright holders may be indicated within.
*
* The entirety of this work is licensed under the Apache License,
Expand All @@ -26,12 +26,6 @@ import (
"github.com/DataWorkflowServices/dws/utils/updater"
)

// SystemConfigurationComputeNode describes a compute node in the system
type SystemConfigurationComputeNode struct {
// Name of the compute node
Name string `json:"name"`
}

// SystemConfigurationComputeNodeReference describes a compute node that
// has access to a server.
type SystemConfigurationComputeNodeReference struct {
Expand All @@ -57,9 +51,6 @@ type SystemConfigurationStorageNode struct {
// SystemConfigurationSpec describes the node layout of the system. This is filled in by
// an administrator at software installation time.
type SystemConfigurationSpec struct {
// ComputeNodes is the list of compute nodes on the system
ComputeNodes []SystemConfigurationComputeNode `json:"computeNodes,omitempty"`

// StorageNodes is the list of storage nodes on the system
StorageNodes []SystemConfigurationStorageNode `json:"storageNodes,omitempty"`

Expand Down Expand Up @@ -113,3 +104,13 @@ type SystemConfigurationList struct {
func init() {
SchemeBuilder.Register(&SystemConfiguration{}, &SystemConfigurationList{})
}

func (in *SystemConfiguration) Computes() []string {
computes := make([]string, 0)
for _, storageNode := range in.Spec.StorageNodes {
for _, computeNode := range storageNode.ComputesAccess {
computes = append(computes, computeNode.Name)
}
}
return computes
}
20 changes: 0 additions & 20 deletions api/v1alpha2/zz_generated.deepcopy.go

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

Original file line number Diff line number Diff line change
Expand Up @@ -152,19 +152,6 @@ spec:
system. This is filled in by an administrator at software installation
time.
properties:
computeNodes:
description: ComputeNodes is the list of compute nodes on the system
items:
description: SystemConfigurationComputeNode describes a compute
node in the system
properties:
name:
description: Name of the compute node
type: string
required:
- name
type: object
type: array
ports:
description: Ports is the list of ports available for communication
between nodes in the system. Valid values are single integers, or
Expand Down

0 comments on commit 3985b6f

Please sign in to comment.