From 5a7934c61cc5f2739d90207a1a06643aab923186 Mon Sep 17 00:00:00 2001 From: Taylor Neyland <57606775+taneyland@users.noreply.github.com> Date: Mon, 9 Oct 2023 09:31:33 -0500 Subject: [PATCH] Removing ethtool DS (#6785) --- .../vsphere/config/ethtool_daemonset.yaml | 33 --- pkg/providers/vsphere/mocks/client.go | 44 +--- pkg/providers/vsphere/vsphere.go | 66 ------ pkg/providers/vsphere/vsphere_test.go | 200 ------------------ 4 files changed, 4 insertions(+), 339 deletions(-) delete mode 100644 pkg/providers/vsphere/config/ethtool_daemonset.yaml diff --git a/pkg/providers/vsphere/config/ethtool_daemonset.yaml b/pkg/providers/vsphere/config/ethtool_daemonset.yaml deleted file mode 100644 index 410163a659f9..000000000000 --- a/pkg/providers/vsphere/config/ethtool_daemonset.yaml +++ /dev/null @@ -1,33 +0,0 @@ -apiVersion: apps/v1 -kind: DaemonSet -metadata: - name: vsphere-disable-udp-offload - namespace: {{.eksaSystemNamespace}} -spec: - selector: - matchLabels: - name: vsphere-disable-udp-offload - template: - metadata: - labels: - name: vsphere-disable-udp-offload - spec: - containers: - - name: vsphere-disable-udp-offload-complete - image: {{.kindNodeImage}} - imagePullPolicy: IfNotPresent - command: [ "/bin/sh" ] - args: [ "-c", "sleep infinity" ] - initContainers: - - name: vsphere-disable-udp-offload - image: {{.kindNodeImage}} - command: - - "/bin/sh" - - "-c" - - "ethtool -K eth0 tx-udp_tnl-segmentation off && ethtool -K eth0 tx-udp_tnl-csum-segmentation off && echo 'done'" - securityContext: - privileged: true - hostNetwork: true - restartPolicy: Always - tolerations: - - operator: "Exists" \ No newline at end of file diff --git a/pkg/providers/vsphere/mocks/client.go b/pkg/providers/vsphere/mocks/client.go index ac5ff8d60e7b..de6cb007c3cd 100644 --- a/pkg/providers/vsphere/mocks/client.go +++ b/pkg/providers/vsphere/mocks/client.go @@ -9,14 +9,12 @@ import ( reflect "reflect" v1alpha1 "github.com/aws/eks-anywhere/pkg/api/v1alpha1" - kubernetes "github.com/aws/eks-anywhere/pkg/clients/kubernetes" executables "github.com/aws/eks-anywhere/pkg/executables" govmomi "github.com/aws/eks-anywhere/pkg/govmomi" types "github.com/aws/eks-anywhere/pkg/types" v1beta1 "github.com/aws/etcdadm-controller/api/v1beta1" gomock "github.com/golang/mock/gomock" - v1 "k8s.io/api/apps/v1" - v10 "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" v1beta10 "sigs.k8s.io/cluster-api/api/v1beta1" v1beta11 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1beta1" ) @@ -580,7 +578,7 @@ func (mr *MockProviderKubectlClientMockRecorder) ApplyKubeSpecFromBytes(arg0, ar } // ApplyTolerationsFromTaintsToDaemonSet mocks base method. -func (m *MockProviderKubectlClient) ApplyTolerationsFromTaintsToDaemonSet(arg0 context.Context, arg1, arg2 []v10.Taint, arg3, arg4 string) error { +func (m *MockProviderKubectlClient) ApplyTolerationsFromTaintsToDaemonSet(arg0 context.Context, arg1, arg2 []v1.Taint, arg3, arg4 string) error { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "ApplyTolerationsFromTaintsToDaemonSet", arg0, arg1, arg2, arg3, arg4) ret0, _ := ret[0].(error) @@ -607,25 +605,6 @@ func (mr *MockProviderKubectlClientMockRecorder) CreateNamespaceIfNotPresent(arg return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateNamespaceIfNotPresent", reflect.TypeOf((*MockProviderKubectlClient)(nil).CreateNamespaceIfNotPresent), arg0, arg1, arg2) } -// Delete mocks base method. -func (m *MockProviderKubectlClient) Delete(arg0 context.Context, arg1, arg2 string, arg3 ...kubernetes.KubectlDeleteOption) error { - m.ctrl.T.Helper() - varargs := []interface{}{arg0, arg1, arg2} - for _, a := range arg3 { - varargs = append(varargs, a) - } - ret := m.ctrl.Call(m, "Delete", varargs...) - ret0, _ := ret[0].(error) - return ret0 -} - -// Delete indicates an expected call of Delete. -func (mr *MockProviderKubectlClientMockRecorder) Delete(arg0, arg1, arg2 interface{}, arg3 ...interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - varargs := append([]interface{}{arg0, arg1, arg2}, arg3...) - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Delete", reflect.TypeOf((*MockProviderKubectlClient)(nil).Delete), varargs...) -} - // DeleteEksaDatacenterConfig mocks base method. func (m *MockProviderKubectlClient) DeleteEksaDatacenterConfig(arg0 context.Context, arg1, arg2, arg3, arg4 string) error { m.ctrl.T.Helper() @@ -654,21 +633,6 @@ func (mr *MockProviderKubectlClientMockRecorder) DeleteEksaMachineConfig(arg0, a return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteEksaMachineConfig", reflect.TypeOf((*MockProviderKubectlClient)(nil).DeleteEksaMachineConfig), arg0, arg1, arg2, arg3, arg4) } -// GetDaemonSet mocks base method. -func (m *MockProviderKubectlClient) GetDaemonSet(arg0 context.Context, arg1, arg2, arg3 string) (*v1.DaemonSet, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetDaemonSet", arg0, arg1, arg2, arg3) - ret0, _ := ret[0].(*v1.DaemonSet) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GetDaemonSet indicates an expected call of GetDaemonSet. -func (mr *MockProviderKubectlClientMockRecorder) GetDaemonSet(arg0, arg1, arg2, arg3 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetDaemonSet", reflect.TypeOf((*MockProviderKubectlClient)(nil).GetDaemonSet), arg0, arg1, arg2, arg3) -} - // GetEksaCluster mocks base method. func (m *MockProviderKubectlClient) GetEksaCluster(arg0 context.Context, arg1 *types.Cluster, arg2 string) (*v1alpha1.Cluster, error) { m.ctrl.T.Helper() @@ -775,10 +739,10 @@ func (mr *MockProviderKubectlClientMockRecorder) GetMachineDeployment(arg0, arg1 } // GetSecretFromNamespace mocks base method. -func (m *MockProviderKubectlClient) GetSecretFromNamespace(arg0 context.Context, arg1, arg2, arg3 string) (*v10.Secret, error) { +func (m *MockProviderKubectlClient) GetSecretFromNamespace(arg0 context.Context, arg1, arg2, arg3 string) (*v1.Secret, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetSecretFromNamespace", arg0, arg1, arg2, arg3) - ret0, _ := ret[0].(*v10.Secret) + ret0, _ := ret[0].(*v1.Secret) ret1, _ := ret[1].(error) return ret0, ret1 } diff --git a/pkg/providers/vsphere/vsphere.go b/pkg/providers/vsphere/vsphere.go index 5eccc83030f1..8503d19f69fc 100644 --- a/pkg/providers/vsphere/vsphere.go +++ b/pkg/providers/vsphere/vsphere.go @@ -12,15 +12,12 @@ import ( "github.com/Masterminds/sprig" etcdv1 "github.com/aws/etcdadm-controller/api/v1beta1" - appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1beta1" "github.com/aws/eks-anywhere/pkg/api/v1alpha1" "github.com/aws/eks-anywhere/pkg/bootstrapper" - "github.com/aws/eks-anywhere/pkg/clients/kubernetes" "github.com/aws/eks-anywhere/pkg/cluster" "github.com/aws/eks-anywhere/pkg/config" "github.com/aws/eks-anywhere/pkg/constants" @@ -31,7 +28,6 @@ import ( "github.com/aws/eks-anywhere/pkg/providers" "github.com/aws/eks-anywhere/pkg/providers/common" "github.com/aws/eks-anywhere/pkg/retrier" - "github.com/aws/eks-anywhere/pkg/templater" "github.com/aws/eks-anywhere/pkg/types" "github.com/aws/eks-anywhere/pkg/validations" releasev1alpha1 "github.com/aws/eks-anywhere/release/api/v1alpha1" @@ -53,7 +49,6 @@ const ( disk1 = "Hard disk 1" disk2 = "Hard disk 2" MemoryAvailable = "Memory_Available" - ethtoolDaemonSetName = "vsphere-disable-udp-offload" ) //go:embed config/template-cp.yaml @@ -65,9 +60,6 @@ var defaultClusterConfigMD string //go:embed config/secret.yaml var defaultSecretObject string -//go:embed config/ethtool_daemonset.yaml -var ethtoolDaemonSetObject string - var ( eksaVSphereDatacenterResourceType = fmt.Sprintf("vspheredatacenterconfigs.%s", v1alpha1.GroupVersion.Group) eksaVSphereMachineResourceType = fmt.Sprintf("vspheremachineconfigs.%s", v1alpha1.GroupVersion.Group) @@ -145,8 +137,6 @@ type ProviderKubectlClient interface { DeleteEksaDatacenterConfig(ctx context.Context, vsphereDatacenterResourceType string, vsphereDatacenterConfigName string, kubeconfigFile string, namespace string) error DeleteEksaMachineConfig(ctx context.Context, vsphereMachineResourceType string, vsphereMachineConfigName string, kubeconfigFile string, namespace string) error ApplyTolerationsFromTaintsToDaemonSet(ctx context.Context, oldTaints []corev1.Taint, newTaints []corev1.Taint, dsName string, kubeconfigFile string) error - GetDaemonSet(ctx context.Context, name, namespace, kubeconfig string) (*appsv1.DaemonSet, error) - Delete(ctx context.Context, resourceType, kubeconfig string, opts ...kubernetes.KubectlDeleteOption) error } // IPValidator is an interface that defines methods to validate the control plane IP. @@ -1271,15 +1261,6 @@ func (p *vsphereProvider) InstallCustomProviderComponents(ctx context.Context, k // PostBootstrapDeleteForUpgrade runs any provider-specific operations after bootstrap cluster has been deleted. func (p *vsphereProvider) PostBootstrapDeleteForUpgrade(ctx context.Context, cluster *types.Cluster) error { - // Delete the daemonset that was used to disable udp offloading in ubuntu/redhat nodes. - logger.V(3).Info("Deleting vsphere-disable-udp-offload daemonset") - o := &kubernetes.KubectlDeleteOptions{ - Name: ethtoolDaemonSetName, - Namespace: constants.EksaSystemNamespace, - } - if err := p.providerKubectlClient.Delete(ctx, "daemonset", cluster.KubeconfigFile, o); err != nil && !apierrors.IsNotFound(err) { - return fmt.Errorf("deleting daemonset: %v", err) - } return nil } @@ -1289,52 +1270,5 @@ func (p *vsphereProvider) PreCoreComponentsUpgrade( cluster *types.Cluster, clusterSpec *cluster.Spec, ) error { - // This is only needed for EKS-A v0.17 because UDP offloading needs to be disabled for - // the new Cilium version to work with VSphere clusters with Redhat OS family. Since our templates that were shipped - // in v0.16 releases still have UDP offloading enabled in the nodes, we must apply the daemon set - // to disable UDP offloading in all the nodes before upgrading Cilium. - // We will remove this in EKS-A release v0.18.0. - return p.applyEthtoolDaemonSet(ctx, cluster, clusterSpec) -} - -func (p *vsphereProvider) applyEthtoolDaemonSet(ctx context.Context, cluster *types.Cluster, clusterSpec *cluster.Spec) error { - for _, mc := range clusterSpec.Config.VSphereMachineConfigs { - // This is only needed for Redhat OS. - // We only need to check one since the OS family has to be the same for all machine configs - if mc.Spec.OSFamily != v1alpha1.RedHat { - return nil - } - } - logger.V(4).Info("Applying vsphere-disable-udp-offload daemonset") - bundle := clusterSpec.RootVersionsBundle() - values := map[string]interface{}{ - "eksaSystemNamespace": constants.EksaSystemNamespace, - "kindNodeImage": bundle.EksD.KindNode.VersionedImage(), - } - b, err := templater.Execute(ethtoolDaemonSetObject, values) - if err != nil { - return err - } - - if err = p.providerKubectlClient.ApplyKubeSpecFromBytes(ctx, cluster, b); err != nil { - return err - } - - return p.Retrier.Retry( - func() error { - return p.checkEthtoolDaemonSetReady(ctx, cluster) - }, - ) -} - -func (p *vsphereProvider) checkEthtoolDaemonSetReady(ctx context.Context, cluster *types.Cluster) error { - ethtoolDaemonSet, err := p.providerKubectlClient.GetDaemonSet(ctx, ethtoolDaemonSetName, constants.EksaSystemNamespace, cluster.KubeconfigFile) - if err != nil { - return err - } - - if ethtoolDaemonSet.Status.DesiredNumberScheduled != ethtoolDaemonSet.Status.NumberReady { - return fmt.Errorf("daemonSet %s is not ready: %d/%d ready", ethtoolDaemonSetName, ethtoolDaemonSet.Status.NumberReady, ethtoolDaemonSet.Status.DesiredNumberScheduled) - } return nil } diff --git a/pkg/providers/vsphere/vsphere_test.go b/pkg/providers/vsphere/vsphere_test.go index fa2f5e2df6f4..46d050d53f3e 100644 --- a/pkg/providers/vsphere/vsphere_test.go +++ b/pkg/providers/vsphere/vsphere_test.go @@ -20,7 +20,6 @@ import ( "github.com/golang/mock/gomock" . "github.com/onsi/gomega" "github.com/stretchr/testify/assert" - appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" @@ -36,7 +35,6 @@ import ( "github.com/aws/eks-anywhere/pkg/govmomi" govmomi_mocks "github.com/aws/eks-anywhere/pkg/govmomi/mocks" "github.com/aws/eks-anywhere/pkg/providers/vsphere/mocks" - "github.com/aws/eks-anywhere/pkg/retrier" "github.com/aws/eks-anywhere/pkg/types" "github.com/aws/eks-anywhere/pkg/utils/ptr" releasev1alpha1 "github.com/aws/eks-anywhere/release/api/v1alpha1" @@ -3952,201 +3950,3 @@ func TestProviderGenerateDeploymentFileForBottlerocketWithTrustedCertBundles(t * test.AssertContentToFile(t, string(cp), "testdata/expected_results_bottlerocket_cert_bundles_config_cp.yaml") test.AssertContentToFile(t, string(md), "testdata/expected_results_bottlerocket_cert_bundles_config_md.yaml") } - -func TestPreCoreComponentsUpgradeSuccess(t *testing.T) { - ctx := context.Background() - datacenterConfig := givenDatacenterConfig(t, testClusterConfigRedhatFilename) - clusterConfig := givenClusterConfig(t, testClusterConfigRedhatFilename) - clusterSpec := givenClusterSpec(t, testClusterConfigRedhatFilename) - mockCtrl := gomock.NewController(t) - kubectl := mocks.NewMockProviderKubectlClient(mockCtrl) - ipValidator := mocks.NewMockIPValidator(mockCtrl) - provider := newProviderWithKubectl(t, datacenterConfig, clusterConfig, kubectl, ipValidator) - cluster := &types.Cluster{ - Name: "test", - KubeconfigFile: "", - } - values := map[string]string{ - "kindNodeImage": "test", - "eksaSystemNamespace": constants.EksaSystemNamespace, - } - daemonSet := &appsv1.DaemonSet{ - ObjectMeta: metav1.ObjectMeta{ - Name: "ds", - }, - Status: appsv1.DaemonSetStatus{ - DesiredNumberScheduled: 5, - NumberReady: 5, - }, - } - - setupContext(t) - - kubectl.EXPECT().ApplyKubeSpecFromBytes(ctx, cluster, gomock.Any()) - kubectl.EXPECT().GetDaemonSet(ctx, ethtoolDaemonSetName, constants.EksaSystemNamespace, gomock.Any()).Return(daemonSet, nil) - - template, err := template.New("test").Funcs(sprig.TxtFuncMap()).Parse(ethtoolDaemonSetObject) - if err != nil { - t.Fatalf("template create error: %v", err) - } - err = template.Execute(&bytes.Buffer{}, values) - if err != nil { - t.Fatalf("template execute error: %v", err) - } - - err = provider.PreCoreComponentsUpgrade(ctx, cluster, clusterSpec) - if err != nil { - t.Fatalf("PreCoreComponentsUpgrade error %v", err) - } -} - -func TestPreCoreComponentsUpgradeBottlerocketNoOpSuccess(t *testing.T) { - ctx := context.Background() - clusterSpecManifest := "cluster_bottlerocket_external_etcd.yaml" - datacenterConfig := givenDatacenterConfig(t, testClusterConfigMainFilename) - clusterConfig := givenClusterConfig(t, testClusterConfigMainFilename) - clusterSpec := givenClusterSpec(t, clusterSpecManifest) - mockCtrl := gomock.NewController(t) - kubectl := mocks.NewMockProviderKubectlClient(mockCtrl) - ipValidator := mocks.NewMockIPValidator(mockCtrl) - provider := newProviderWithKubectl(t, datacenterConfig, clusterConfig, kubectl, ipValidator) - cluster := &types.Cluster{ - Name: "test", - KubeconfigFile: "", - } - - setupContext(t) - - // No EXPECTs because this should not execute for Bottlerocket - - err := provider.PreCoreComponentsUpgrade(ctx, cluster, clusterSpec) - if err != nil { - t.Fatalf("PreCoreComponentsUpgrade error %v", err) - } -} - -func TestPreCoreComponentsUpgradeApplyError(t *testing.T) { - ctx := context.Background() - datacenterConfig := givenDatacenterConfig(t, testClusterConfigRedhatFilename) - clusterConfig := givenClusterConfig(t, testClusterConfigRedhatFilename) - clusterSpec := givenClusterSpec(t, testClusterConfigRedhatFilename) - mockCtrl := gomock.NewController(t) - kubectl := mocks.NewMockProviderKubectlClient(mockCtrl) - ipValidator := mocks.NewMockIPValidator(mockCtrl) - provider := newProviderWithKubectl(t, datacenterConfig, clusterConfig, kubectl, ipValidator) - cluster := &types.Cluster{ - Name: "test", - KubeconfigFile: "", - } - values := map[string]string{ - "kindNodeImage": "test", - "eksaSystemNamespace": constants.EksaSystemNamespace, - } - - setupContext(t) - - kubectl.EXPECT().ApplyKubeSpecFromBytes(ctx, cluster, gomock.Any()).Return(errors.New("error in apply")) - - template, err := template.New("test").Funcs(sprig.TxtFuncMap()).Parse(ethtoolDaemonSetObject) - if err != nil { - t.Fatalf("template create error: %v", err) - } - err = template.Execute(&bytes.Buffer{}, values) - if err != nil { - t.Fatalf("template execute error: %v", err) - } - - err = provider.PreCoreComponentsUpgrade(ctx, cluster, clusterSpec) - if err == nil || !strings.Contains(err.Error(), "error in apply") { - t.Errorf("expected \"error in apply\" error, got %s", err) - } -} - -func TestPreCoreComponentsUpgradeDSNotReadyError(t *testing.T) { - ctx := context.Background() - datacenterConfig := givenDatacenterConfig(t, testClusterConfigRedhatFilename) - clusterConfig := givenClusterConfig(t, testClusterConfigRedhatFilename) - clusterSpec := givenClusterSpec(t, testClusterConfigRedhatFilename) - mockCtrl := gomock.NewController(t) - kubectl := mocks.NewMockProviderKubectlClient(mockCtrl) - ipValidator := mocks.NewMockIPValidator(mockCtrl) - provider := newProviderWithKubectl(t, datacenterConfig, clusterConfig, kubectl, ipValidator) - provider.Retrier = retrier.NewWithMaxRetries(2, 0) - cluster := &types.Cluster{ - Name: "test", - KubeconfigFile: "", - } - values := map[string]string{ - "kindNodeImage": "test", - "eksaSystemNamespace": constants.EksaSystemNamespace, - } - daemonSet := &appsv1.DaemonSet{ - ObjectMeta: metav1.ObjectMeta{ - Name: "ds", - }, - Status: appsv1.DaemonSetStatus{ - DesiredNumberScheduled: 5, - NumberReady: 3, - }, - } - - setupContext(t) - - kubectl.EXPECT().ApplyKubeSpecFromBytes(ctx, cluster, gomock.Any()) - kubectl.EXPECT().GetDaemonSet(ctx, ethtoolDaemonSetName, constants.EksaSystemNamespace, gomock.Any()).Times(2).Return(daemonSet, nil) - - template, err := template.New("test").Funcs(sprig.TxtFuncMap()).Parse(ethtoolDaemonSetObject) - if err != nil { - t.Fatalf("template create error: %v", err) - } - err = template.Execute(&bytes.Buffer{}, values) - if err != nil { - t.Fatalf("template execute error: %v", err) - } - - notReadyErr := "not ready: 3/5 ready" - err = provider.PreCoreComponentsUpgrade(ctx, cluster, clusterSpec) - if err == nil || !strings.Contains(err.Error(), notReadyErr) { - t.Errorf("expected %s error, got %v", notReadyErr, err) - } -} - -func TestPostBootstrapDeleteForUpgradeSuccess(t *testing.T) { - ctx := context.Background() - datacenterConfig := givenDatacenterConfig(t, testClusterConfigRedhatFilename) - clusterConfig := givenClusterConfig(t, testClusterConfigRedhatFilename) - mockCtrl := gomock.NewController(t) - kubectl := mocks.NewMockProviderKubectlClient(mockCtrl) - ipValidator := mocks.NewMockIPValidator(mockCtrl) - provider := newProviderWithKubectl(t, datacenterConfig, clusterConfig, kubectl, ipValidator) - cluster := &types.Cluster{ - Name: "test", - KubeconfigFile: "", - } - - kubectl.EXPECT().Delete(ctx, "daemonset", gomock.Any(), gomock.Any()) - - err := provider.PostBootstrapDeleteForUpgrade(ctx, cluster) - assert.NoError(t, err) -} - -func TestPostBootstrapDeleteForUpgradeError(t *testing.T) { - ctx := context.Background() - datacenterConfig := givenDatacenterConfig(t, testClusterConfigRedhatFilename) - clusterConfig := givenClusterConfig(t, testClusterConfigRedhatFilename) - mockCtrl := gomock.NewController(t) - kubectl := mocks.NewMockProviderKubectlClient(mockCtrl) - ipValidator := mocks.NewMockIPValidator(mockCtrl) - provider := newProviderWithKubectl(t, datacenterConfig, clusterConfig, kubectl, ipValidator) - cluster := &types.Cluster{ - Name: "test", - KubeconfigFile: "", - } - - kubectl.EXPECT().Delete(ctx, "daemonset", gomock.Any(), gomock.Any()).Return(errors.New("delete error")) - - err := provider.PostBootstrapDeleteForUpgrade(ctx, cluster) - if err == nil || !strings.Contains(err.Error(), "delete error") { - t.Errorf("expected \"delete error\" error, got %s", err) - } -}