From 80d7d19b9f6ed78b74360ed81097fd78eb473ed0 Mon Sep 17 00:00:00 2001 From: Heba Elayoty <31887807+helayoty@users.noreply.github.com> Date: Mon, 23 Jan 2023 21:18:01 -0800 Subject: [PATCH] refactor: Remove custom code used by old Azure SDK - part 2 (#423) Signed-off-by: Heba Elayoty --- pkg/client/client.go | 143 ------------------------ pkg/client/client_apis.go | 6 +- pkg/client/retry.go | 60 ---------- pkg/metrics/mock_metrics_test.go | 5 +- pkg/network/aci_network.go | 11 +- pkg/provider/aci.go | 45 ++++---- pkg/provider/aci_init_container_test.go | 8 +- pkg/provider/aci_test.go | 41 ++++--- pkg/provider/aci_volumes_test.go | 31 +++-- pkg/provider/mock_aci_test.go | 5 +- 10 files changed, 71 insertions(+), 284 deletions(-) delete mode 100644 pkg/client/client.go delete mode 100644 pkg/client/retry.go diff --git a/pkg/client/client.go b/pkg/client/client.go deleted file mode 100644 index 2523c46e..00000000 --- a/pkg/client/client.go +++ /dev/null @@ -1,143 +0,0 @@ -package client - -import ( - "context" - "encoding/json" - "fmt" - "net/http" - - azaciv2 "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/containerinstance/armcontainerinstance/v2" - azaci "github.com/Azure/azure-sdk-for-go/services/containerinstance/mgmt/2021-10-01/containerinstance" - - "github.com/Azure/go-autorest/autorest" - "github.com/virtual-kubelet/virtual-kubelet/log" -) - -const ( - APIVersion = "2021-10-01" - containerGroupURLPath = "/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.ContainerInstance/containerGroups/{containerGroupName}" -) - -type ContainerGroupPropertiesWrapper struct { - ContainerGroupProperties *azaciv2.ContainerGroupProperties - Extensions []*azaciv2.DeploymentExtensionSpec -} - -type ContainerGroupWrapper struct { - autorest.Response `json:"-"` - // Identity - The identity of the container group, if configured. - Identity *azaciv2.ContainerGroupIdentity `json:"identity,omitempty"` - // ContainerGroupProperties - The container group properties - ContainerGroupPropertiesWrapper *ContainerGroupPropertiesWrapper `json:"properties,omitempty"` - // ID - READ-ONLY; The resource id. - ID *string `json:"id,omitempty"` - // Name - READ-ONLY; The resource name. - Name *string `json:"name,omitempty"` - // Type - READ-ONLY; The resource type. - Type *string `json:"type,omitempty"` - // Location - The resource location. - Location *string `json:"location,omitempty"` - // Tags - The resource tags. - Tags map[string]*string `json:"tags"` - // Zones - The zones for the container group. - Zones *[]string `json:"zones,omitempty"` -} - -type ContainerGroupsClientWrapper struct { - CGClient azaci.ContainerGroupsClient -} - -func (c *ContainerGroupsClientWrapper) CreateCG(ctx context.Context, resourceGroupName string, containerGroup ContainerGroupWrapper) error { - logger := log.G(ctx).WithField("method", "CreateCG") - - addReq, err := c.createOrUpdatePreparerWrapper(ctx, resourceGroupName, *containerGroup.Name, containerGroup) - if err != nil { - return err - } - - result, err := c.CGClient.CreateOrUpdateSender(addReq) - logger.Infof("CreateCG status code: %s", result.Status()) - - if err != nil { - err = autorest.NewErrorWithError(err, "containerinstance.ContainerGroupsClient", "CreateOrUpdateSender", result.Response(), "Failure sending request") - return err - } - - // 200 (OK) and 201 (Created) are a successful responses. - if result.Response() != nil { - if result.Response().StatusCode != http.StatusOK && result.Response().StatusCode != http.StatusCreated { - return fmt.Errorf("failed to create container group %s, status code %d ", *containerGroup.Name, result.Response().StatusCode) - } - } - - logger.Infof("container group %s has created successfully", *containerGroup.Name) - return nil -} - -// createOrUpdatePreparerWrapper prepares the CreateOrUpdateSender request for the wrapper. -func (c *ContainerGroupsClientWrapper) createOrUpdatePreparerWrapper(ctx context.Context, resourceGroupName string, containerGroupName string, containerGroup ContainerGroupWrapper) (*http.Request, error) { - pathParameters := map[string]interface{}{ - "containerGroupName": autorest.Encode("path", containerGroupName), - "resourceGroupName": autorest.Encode("path", resourceGroupName), - "subscriptionId": autorest.Encode("path", c.CGClient.SubscriptionID), - } - - queryParameters := map[string]interface{}{ - "api-version": APIVersion, - } - - preparer := autorest.CreatePreparer( - autorest.AsContentType("application/json; charset=utf-8"), - autorest.AsPut(), - autorest.WithBaseURL(c.CGClient.BaseURI), - autorest.WithPathParameters(containerGroupURLPath, pathParameters), - autorest.WithJSON(containerGroup), - autorest.WithQueryParameters(queryParameters)) - - return preparer.Prepare((&http.Request{}).WithContext(ctx)) -} - -// MarshalJSON is the custom marshal for ContainerGroupProperties. -func (cg ContainerGroupPropertiesWrapper) MarshalJSON() ([]byte, error) { - objectMap := make(map[string]interface{}) - if cg.ContainerGroupProperties.Properties.Containers != nil { - objectMap["containers"] = cg.ContainerGroupProperties.Properties.Containers - } - if cg.ContainerGroupProperties.Properties.ImageRegistryCredentials != nil { - objectMap["imageRegistryCredentials"] = cg.ContainerGroupProperties.Properties.ImageRegistryCredentials - } - if cg.ContainerGroupProperties.Properties.RestartPolicy != nil { - objectMap["restartPolicy"] = cg.ContainerGroupProperties.Properties.RestartPolicy - } - if cg.ContainerGroupProperties.Properties.IPAddress != nil { - objectMap["ipAddress"] = cg.ContainerGroupProperties.Properties.IPAddress - } - if cg.ContainerGroupProperties.Properties.OSType != nil { - objectMap["osType"] = cg.ContainerGroupProperties.Properties.OSType - } - if cg.ContainerGroupProperties.Properties.Volumes != nil { - objectMap["volumes"] = cg.ContainerGroupProperties.Properties.Volumes - } - if cg.ContainerGroupProperties.Properties.Diagnostics != nil { - objectMap["diagnostics"] = cg.ContainerGroupProperties.Properties.Diagnostics - } - if cg.ContainerGroupProperties.Properties.SubnetIDs != nil { - objectMap["subnetIds"] = cg.ContainerGroupProperties.Properties.SubnetIDs - } - if cg.ContainerGroupProperties.Properties.DNSConfig != nil { - objectMap["dnsConfig"] = cg.ContainerGroupProperties.Properties.DNSConfig - } - if cg.ContainerGroupProperties.Properties.SKU != nil { - objectMap["sku"] = cg.ContainerGroupProperties.Properties.SKU - } - if cg.ContainerGroupProperties.Properties.EncryptionProperties != nil { - objectMap["encryptionProperties"] = cg.ContainerGroupProperties.Properties.EncryptionProperties - } - if cg.ContainerGroupProperties.Properties.InitContainers != nil { - objectMap["initContainers"] = cg.ContainerGroupProperties.Properties.InitContainers - } - if cg.Extensions != nil { - objectMap["extensions"] = cg.Extensions - } - return json.Marshal(objectMap) -} diff --git a/pkg/client/client_apis.go b/pkg/client/client_apis.go index 1c419f70..69c78d01 100644 --- a/pkg/client/client_apis.go +++ b/pkg/client/client_apis.go @@ -21,7 +21,7 @@ import ( type AzClientsInterface interface { ContainerGroupGetter - CreateContainerGroup(ctx context.Context, resourceGroup, podNS, podName string, cg *ContainerGroupWrapper) error + CreateContainerGroup(ctx context.Context, resourceGroup, podNS, podName string, cg *azaciv2.ContainerGroup) error GetContainerGroupInfo(ctx context.Context, resourceGroup, namespace, name, nodeName string) (*azaciv2.ContainerGroup, error) GetContainerGroupListResult(ctx context.Context, resourceGroup string) ([]*azaciv2.ContainerGroup, error) ListCapabilities(ctx context.Context, region string) ([]*azaciv2.Capabilities, error) @@ -113,14 +113,14 @@ func (a *AzClientsAPIs) GetContainerGroup(ctx context.Context, resourceGroup, co return &result.ContainerGroup, nil } -func (a *AzClientsAPIs) CreateContainerGroup(ctx context.Context, resourceGroup, podNS, podName string, cg *ContainerGroupWrapper) error { +func (a *AzClientsAPIs) CreateContainerGroup(ctx context.Context, resourceGroup, podNS, podName string, cg *azaciv2.ContainerGroup) error { logger := log.G(ctx).WithField("method", "CreateContainerGroup") ctx, span := trace.StartSpan(ctx, "client.CreateContainerGroup") defer span.End() cgName := containerGroupName(podNS, podName) containerGroup := azaciv2.ContainerGroup{ - Properties: cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Properties, + Properties: cg.Properties, Name: &cgName, Type: cg.Type, Identity: cg.Identity, diff --git a/pkg/client/retry.go b/pkg/client/retry.go deleted file mode 100644 index 94fe1af9..00000000 --- a/pkg/client/retry.go +++ /dev/null @@ -1,60 +0,0 @@ -package client - -import ( - "fmt" - "os" - "strconv" - "time" -) - -const ( - // DefaultRetryIntervalMin - the default minimum retry wait interval - DefaultRetryIntervalMin = 1 * time.Second - // DefaultRetryIntervalMax - the default maximum retry wait interval - DefaultRetryIntervalMax = 60 * time.Second - // DefaultRetryMax - the default retry max count - DefaultRetryMax = 40 -) - -// HTTPRetryConfig - retry config for http requests -type HTTPRetryConfig struct { - RetryWaitMin time.Duration - RetryWaitMax time.Duration - RetryMax int -} - -func SetupRetry() (*HTTPRetryConfig, error) { - retryWaitMin := DefaultRetryIntervalMin - if value := os.Getenv("RETRY_MINIMUM_INTERVAL_IN_SECOND"); value != "" { - ret, err := strconv.Atoi(value) - if err == nil { - - return nil, fmt.Errorf("env RETRY_MINIMUM_INTERVAL_IN_SECOND is not able to convert to int, err: %s", err) - } - retryWaitMin = time.Duration(ret) * time.Second - } - - retryWaitMax := DefaultRetryIntervalMax - if value := os.Getenv("RETRY_MAXIMUM_INTERVAL_IN_SECOND"); value != "" { - ret, err := strconv.Atoi(value) - if err != nil { - return nil, fmt.Errorf("env RETRY_MAXIMUM_INTERVAL_IN_SECOND is not able to convert to int, err: %s", err) - } - retryWaitMax = time.Duration(ret) * time.Second - } - - retryMax := DefaultRetryMax - if value := os.Getenv("RETRY_MAXIMUM_COUNT"); value != "" { - ret, err := strconv.Atoi(value) - if err != nil { - return nil, fmt.Errorf("env RETRY_MAXIMUM_COUNT is not able to convert to int, err: %s", err) - } - retryMax = ret - } - - return &HTTPRetryConfig{ - RetryWaitMin: retryWaitMin, - RetryWaitMax: retryWaitMax, - RetryMax: retryMax, - }, nil -} diff --git a/pkg/metrics/mock_metrics_test.go b/pkg/metrics/mock_metrics_test.go index 0b1b6361..77e88971 100644 --- a/pkg/metrics/mock_metrics_test.go +++ b/pkg/metrics/mock_metrics_test.go @@ -10,7 +10,6 @@ import ( azaciv2 "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/containerinstance/armcontainerinstance/v2" gomock "github.com/golang/mock/gomock" - "github.com/virtual-kubelet/azure-aci/pkg/client" statsv1alpha1 "github.com/virtual-kubelet/virtual-kubelet/node/api/statsv1alpha1" v1 "k8s.io/api/core/v1" ) @@ -152,10 +151,10 @@ func (mr *MockpodStatsGetterMockRecorder) GetPodStats(ctx, pod interface{}) *gom } // getContainerGroupFromPod mocks base method. -func (m *MockpodStatsGetter) getContainerGroupFromPod(ctx context.Context, pod *v1.Pod) (*client.ContainerGroupWrapper, error) { +func (m *MockpodStatsGetter) getContainerGroupFromPod(ctx context.Context, pod *v1.Pod) (*azaciv2.ContainerGroup, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "getContainerGroupFromPod", ctx, pod) - ret0, _ := ret[0].(*client.ContainerGroupWrapper) + ret0, _ := ret[0].(*azaciv2.ContainerGroup) ret1, _ := ret[1].(error) return ret0, ret1 } diff --git a/pkg/network/aci_network.go b/pkg/network/aci_network.go index cdd5e200..1657faea 100644 --- a/pkg/network/aci_network.go +++ b/pkg/network/aci_network.go @@ -20,7 +20,6 @@ import ( azaciv2 "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/containerinstance/armcontainerinstance/v2" aznetwork "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-05-01/network" "github.com/virtual-kubelet/azure-aci/pkg/auth" - client2 "github.com/virtual-kubelet/azure-aci/pkg/client" "github.com/virtual-kubelet/virtual-kubelet/log" v1 "k8s.io/api/core/v1" ) @@ -177,18 +176,18 @@ func (pn *ProviderNetwork) setupNetwork(ctx context.Context, azConfig *auth.Conf return nil } -func (pn *ProviderNetwork) AmendVnetResources(ctx context.Context, cg client2.ContainerGroupWrapper, pod *v1.Pod, clusterDomain string) { +func (pn *ProviderNetwork) AmendVnetResources(ctx context.Context, cg azaciv2.ContainerGroup, pod *v1.Pod, clusterDomain string) { if pn.SubnetName == "" { return } subnetID := "/subscriptions/" + pn.VnetSubscriptionID + "/resourceGroups/" + pn.VnetResourceGroup + "/providers/Microsoft.Network/virtualNetworks/" + pn.VnetName + "/subnets/" + pn.SubnetName cgIDList := []*azaciv2.ContainerGroupSubnetID{{ID: &subnetID}} - cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Properties.SubnetIDs = cgIDList + cg.Properties.SubnetIDs = cgIDList // windows containers don't support DNS config - if cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Properties.OSType != nil && - *cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Properties.OSType != azaciv2.OperatingSystemTypesWindows { - cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Properties.DNSConfig = getDNSConfig(ctx, pod, pn.KubeDNSIP, clusterDomain) + if cg.Properties.OSType != nil && + *cg.Properties.OSType != azaciv2.OperatingSystemTypesWindows { + cg.Properties.DNSConfig = getDNSConfig(ctx, pod, pn.KubeDNSIP, clusterDomain) } } diff --git a/pkg/provider/aci.go b/pkg/provider/aci.go index 1508c8d4..39577618 100644 --- a/pkg/provider/aci.go +++ b/pkg/provider/aci.go @@ -280,20 +280,16 @@ func (p *ACIProvider) CreatePod(ctx context.Context, pod *v1.Pod) error { defer span.End() ctx = addAzureAttributes(ctx, span, p) - cg := &client.ContainerGroupWrapper{ - ContainerGroupPropertiesWrapper: &client.ContainerGroupPropertiesWrapper{ - ContainerGroupProperties: &azaciv2.ContainerGroupProperties{ - Properties: &azaciv2.ContainerGroupPropertiesProperties{}, - }, - }, + cg := &azaciv2.ContainerGroup{ + Properties: &azaciv2.ContainerGroupPropertiesProperties{}, } os := azaciv2.OperatingSystemTypes(p.operatingSystem) policy := azaciv2.ContainerGroupRestartPolicy(pod.Spec.RestartPolicy) cg.Location = &p.region - cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Properties.RestartPolicy = &policy - cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Properties.OSType = &os + cg.Properties.RestartPolicy = &policy + cg.Properties.OSType = &os // get containers containers, err := p.getContainers(pod) @@ -318,14 +314,14 @@ func (p *ACIProvider) CreatePod(ctx context.Context, pod *v1.Pod) error { if err != nil { return err } - cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Properties.InitContainers = initContainers + cg.Properties.InitContainers = initContainers } // assign all the things - cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Properties.Containers = containers - cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Properties.Volumes = volumes - cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Properties.ImageRegistryCredentials = creds - cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Properties.Diagnostics = p.getDiagnostics(pod) + cg.Properties.Containers = containers + cg.Properties.Volumes = volumes + cg.Properties.ImageRegistryCredentials = creds + cg.Properties.Diagnostics = p.getDiagnostics(pod) filterWindowsServiceAccountSecretVolume(ctx, p.operatingSystem, cg) @@ -345,13 +341,13 @@ func (p *ACIProvider) CreatePod(ctx context.Context, pod *v1.Pod) error { } } if len(ports) > 0 && p.providernetwork.SubnetName == "" { - cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Properties.IPAddress = &azaciv2.IPAddress{ + cg.Properties.IPAddress = &azaciv2.IPAddress{ Ports: ports, Type: &util.ContainerGroupIPAddressTypePublic, } if dnsNameLabel := pod.Annotations[virtualKubeletDNSNameLabel]; dnsNameLabel != "" { - cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Properties.IPAddress.DNSNameLabel = &dnsNameLabel + cg.Properties.IPAddress.DNSNameLabel = &dnsNameLabel } } @@ -369,9 +365,9 @@ func (p *ACIProvider) CreatePod(ctx context.Context, pod *v1.Pod) error { p.providernetwork.AmendVnetResources(ctx, *cg, pod, p.clusterDomain) // windows containers don't support kube-proxy nor realtime metrics - if cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Properties.OSType != nil && - *cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Properties.OSType != azaciv2.OperatingSystemTypesWindows { - cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Properties.Extensions = p.containerGroupExtensions + if cg.Properties.OSType != nil && + *cg.Properties.OSType != azaciv2.OperatingSystemTypesWindows { + cg.Properties.Extensions = p.containerGroupExtensions } log.G(ctx).Infof("start creating pod %v", pod.Name) @@ -1165,7 +1161,6 @@ func getProbe(probe *v1.Probe, ports []v1.ContainerPort) (*azaciv2.ContainerProb Command: commands, } } - var httpGET *azaciv2.ContainerHTTPGet if probe.Handler.HTTPGet != nil { var portValue int32 @@ -1208,11 +1203,11 @@ func getProbe(probe *v1.Probe, ports []v1.ContainerPort) (*azaciv2.ContainerProb // Filters service account secret volume for Windows. // Service account secret volume gets automatically turned on if not specified otherwise. // ACI doesn't support secret volume for Windows, so we need to filter it. -func filterWindowsServiceAccountSecretVolume(ctx context.Context, osType string, cgw *client.ContainerGroupWrapper) { +func filterWindowsServiceAccountSecretVolume(ctx context.Context, osType string, cgw *azaciv2.ContainerGroup) { if strings.EqualFold(osType, "Windows") { serviceAccountSecretVolumeName := make(map[string]bool) - for index, container := range cgw.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Properties.Containers { + for index, container := range cgw.Properties.Containers { volumeMounts := make([]*azaciv2.VolumeMount, 0, len(container.Properties.VolumeMounts)) for _, volumeMount := range container.Properties.VolumeMounts { if !strings.EqualFold(serviceAccountSecretMountPath, *volumeMount.MountPath) { @@ -1221,7 +1216,7 @@ func filterWindowsServiceAccountSecretVolume(ctx context.Context, osType string, serviceAccountSecretVolumeName[*volumeMount.Name] = true } } - cgw.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Properties.Containers[index].Properties.VolumeMounts = volumeMounts + cgw.Properties.Containers[index].Properties.VolumeMounts = volumeMounts } if len(serviceAccountSecretVolumeName) == 0 { @@ -1231,14 +1226,14 @@ func filterWindowsServiceAccountSecretVolume(ctx context.Context, osType string, l := log.G(ctx).WithField("containerGroup", cgw.Name) l.Infof("Ignoring service account secret volumes '%v' for Windows", reflect.ValueOf(serviceAccountSecretVolumeName).MapKeys()) - volumes := make([]*azaciv2.Volume, 0, len(cgw.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Properties.Volumes)) - for _, volume := range cgw.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Properties.Volumes { + volumes := make([]*azaciv2.Volume, 0, len(cgw.Properties.Volumes)) + for _, volume := range cgw.Properties.Volumes { if _, ok := serviceAccountSecretVolumeName[*volume.Name]; !ok { volumes = append(volumes, volume) } } - cgw.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Properties.Volumes = volumes + cgw.Properties.Volumes = volumes } } diff --git a/pkg/provider/aci_init_container_test.go b/pkg/provider/aci_init_container_test.go index def043f1..b72ab173 100644 --- a/pkg/provider/aci_init_container_test.go +++ b/pkg/provider/aci_init_container_test.go @@ -4,8 +4,8 @@ import ( "context" "testing" + azaciv2 "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/containerinstance/armcontainerinstance/v2" "github.com/golang/mock/gomock" - "github.com/virtual-kubelet/azure-aci/pkg/client" "github.com/virtual-kubelet/azure-aci/pkg/featureflag" "github.com/virtual-kubelet/node-cli/manager" "github.com/virtual-kubelet/virtual-kubelet/errdefs" @@ -25,9 +25,9 @@ func TestCreatePodWithInitContainers(t *testing.T) { defer mockCtrl.Finish() aciMocks := createNewACIMock() - aciMocks.MockCreateContainerGroup = func(ctx context.Context, resourceGroup, podNS, podName string, cg *client.ContainerGroupWrapper) error { - containers := cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Properties.Containers - initContainers := cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Properties.InitContainers + aciMocks.MockCreateContainerGroup = func(ctx context.Context, resourceGroup, podNS, podName string, cg *azaciv2.ContainerGroup) error { + containers := cg.Properties.Containers + initContainers := cg.Properties.InitContainers assert.Check(t, cg != nil, "Container group is nil") assert.Check(t, containers != nil, "Containers should not be nil") assert.Check(t, initContainers != nil, "Container group is nil") diff --git a/pkg/provider/aci_test.go b/pkg/provider/aci_test.go index 9a2bc65c..868c72d6 100644 --- a/pkg/provider/aci_test.go +++ b/pkg/provider/aci_test.go @@ -17,7 +17,6 @@ import ( "github.com/golang/mock/gomock" "github.com/google/uuid" "github.com/virtual-kubelet/azure-aci/pkg/auth" - "github.com/virtual-kubelet/azure-aci/pkg/client" testsutil "github.com/virtual-kubelet/azure-aci/pkg/tests" "github.com/virtual-kubelet/azure-aci/pkg/util" "github.com/virtual-kubelet/node-cli/manager" @@ -121,8 +120,8 @@ func TestCreatePodWithoutResourceSpec(t *testing.T) { aciMocks := createNewACIMock() - aciMocks.MockCreateContainerGroup = func(ctx context.Context, resourceGroup, podNS, podName string, cg *client.ContainerGroupWrapper) error { - containers := cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Properties.Containers + aciMocks.MockCreateContainerGroup = func(ctx context.Context, resourceGroup, podNS, podName string, cg *azaciv2.ContainerGroup) error { + containers := cg.Properties.Containers assert.Check(t, cg != nil, "Container group is nil") assert.Check(t, containers != nil, "Containers should not be nil") assert.Check(t, is.Equal(1, len(containers)), "1 Container is expected") @@ -162,8 +161,8 @@ func TestCreatePodWithoutResourceSpec(t *testing.T) { func TestCreatePodWithResourceRequestOnly(t *testing.T) { aciMocks := createNewACIMock() - aciMocks.MockCreateContainerGroup = func(ctx context.Context, resourceGroup, podNS, podName string, cg *client.ContainerGroupWrapper) error { - containers := cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Properties.Containers + aciMocks.MockCreateContainerGroup = func(ctx context.Context, resourceGroup, podNS, podName string, cg *azaciv2.ContainerGroup) error { + containers := cg.Properties.Containers assert.Check(t, cg != nil, "container group is nil") assert.Check(t, containers != nil, "container should not be nil") assert.Check(t, is.Equal(1, len(containers)), "only container is expected") @@ -218,8 +217,8 @@ func TestCreatePodWithGPU(t *testing.T) { aciMocks := createNewACIMock() - aciMocks.MockCreateContainerGroup = func(ctx context.Context, resourceGroup, podNS, podName string, cg *client.ContainerGroupWrapper) error { - containers := cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Properties.Containers + aciMocks.MockCreateContainerGroup = func(ctx context.Context, resourceGroup, podNS, podName string, cg *azaciv2.ContainerGroup) error { + containers := cg.Properties.Containers assert.Check(t, containers != nil, "Containers should not be nil") assert.Check(t, is.Equal(1, len(containers)), "1 Container is expected") assert.Check(t, is.Equal("nginx", *(containers[0]).Name), "Container nginx is expected") @@ -272,8 +271,8 @@ func TestCreatePodWithGPUSKU(t *testing.T) { podNamespace := "ns-" + uuid.New().String() aciMocks := createNewACIMock() - aciMocks.MockCreateContainerGroup = func(ctx context.Context, resourceGroup, podNS, podName string, cg *client.ContainerGroupWrapper) error { - containers := cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Properties.Containers + aciMocks.MockCreateContainerGroup = func(ctx context.Context, resourceGroup, podNS, podName string, cg *azaciv2.ContainerGroup) error { + containers := cg.Properties.Containers assert.Check(t, cg != nil, "Container group is nil") assert.Check(t, containers != nil, "Containers should not be nil") assert.Check(t, is.Equal(1, len(containers)), "1 Container is expected") @@ -332,8 +331,8 @@ func TestCreatePodWithResourceRequestAndLimit(t *testing.T) { aciMocks := createNewACIMock() - aciMocks.MockCreateContainerGroup = func(ctx context.Context, resourceGroup, podNS, podName string, cg *client.ContainerGroupWrapper) error { - containers := cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Properties.Containers + aciMocks.MockCreateContainerGroup = func(ctx context.Context, resourceGroup, podNS, podName string, cg *azaciv2.ContainerGroup) error { + containers := cg.Properties.Containers assert.Check(t, cg != nil, "Container group is nil") assert.Check(t, containers != nil, "Containers should not be nil") assert.Check(t, is.Equal(1, len(containers)), "1 Container is expected") @@ -644,8 +643,8 @@ func TestCreatePodWithNamedLivenessProbe(t *testing.T) { aciMocks := createNewACIMock() - aciMocks.MockCreateContainerGroup = func(ctx context.Context, resourceGroup, podNS, podName string, cg *client.ContainerGroupWrapper) error { - containers := cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Properties.Containers + aciMocks.MockCreateContainerGroup = func(ctx context.Context, resourceGroup, podNS, podName string, cg *azaciv2.ContainerGroup) error { + containers := cg.Properties.Containers assert.Check(t, (containers)[0].Properties.LivenessProbe != nil, "Liveness probe expected") assert.Check(t, is.Equal(int32(10), *(containers)[0].Properties.LivenessProbe.InitialDelaySeconds), "Initial Probe Delay doesn't match") @@ -653,7 +652,7 @@ func TestCreatePodWithNamedLivenessProbe(t *testing.T) { assert.Check(t, is.Equal(int32(60), *(containers)[0].Properties.LivenessProbe.TimeoutSeconds), "Probe Timeout doesn't match") assert.Check(t, is.Equal(int32(3), *(containers)[0].Properties.LivenessProbe.SuccessThreshold), "Probe Success Threshold doesn't match") assert.Check(t, is.Equal(int32(5), *(containers)[0].Properties.LivenessProbe.FailureThreshold), "Probe Failure Threshold doesn't match") - assert.Check(t, (cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Properties.Containers)[0].Properties.LivenessProbe.HTTPGet != nil, "Expected an HTTP Get Probe") + assert.Check(t, (cg.Properties.Containers)[0].Properties.LivenessProbe.HTTPGet != nil, "Expected an HTTP Get Probe") assert.Check(t, is.Equal(int32(8080), *(containers)[0].Properties.LivenessProbe.HTTPGet.Port), "Expected Port to be 8080") return nil } @@ -675,8 +674,8 @@ func TestCreatePodWithLivenessProbe(t *testing.T) { podNamespace := "ns-" + uuid.New().String() aciMocks := createNewACIMock() - aciMocks.MockCreateContainerGroup = func(ctx context.Context, resourceGroup, podNS, podName string, cg *client.ContainerGroupWrapper) error { - containers := cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Properties.Containers + aciMocks.MockCreateContainerGroup = func(ctx context.Context, resourceGroup, podNS, podName string, cg *azaciv2.ContainerGroup) error { + containers := cg.Properties.Containers assert.Check(t, cg != nil, "Container group is nil") assert.Check(t, containers != nil, "Containers should not be nil") assert.Check(t, is.Equal(1, len(containers)), "1 Container is expected") @@ -789,8 +788,8 @@ func TestCreatePodWithReadinessProbe(t *testing.T) { aciMocks := createNewACIMock() - aciMocks.MockCreateContainerGroup = func(ctx context.Context, resourceGroup, podNS, podName string, cg *client.ContainerGroupWrapper) error { - containers := cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Properties.Containers + aciMocks.MockCreateContainerGroup = func(ctx context.Context, resourceGroup, podNS, podName string, cg *azaciv2.ContainerGroup) error { + containers := cg.Properties.Containers assert.Check(t, cg != nil, "Container group is nil") assert.Check(t, containers != nil, "Containers should not be nil") assert.Check(t, is.Equal(1, len(containers)), "1 Container is expected") @@ -923,8 +922,8 @@ func TestCreatedPodWithContainerPort(t *testing.T) { pod.Spec.Containers = tc.containerList aciMocks := createNewACIMock() - aciMocks.MockCreateContainerGroup = func(ctx context.Context, resourceGroup, podNS, podName string, cg *client.ContainerGroupWrapper) error { - containers := cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Properties.Containers + aciMocks.MockCreateContainerGroup = func(ctx context.Context, resourceGroup, podNS, podName string, cg *azaciv2.ContainerGroup) error { + containers := cg.Properties.Containers container1Ports := containers[0].Properties.Ports container2Ports := containers[1].Properties.Ports assert.Check(t, cg != nil, "Container group is nil") @@ -987,7 +986,7 @@ func TestGetPodWithContainerID(t *testing.T) { aciMocks.MockGetContainerGroupInfo = func(ctx context.Context, resourceGroup, namespace, name, nodeName string) (*azaciv2.ContainerGroup, error) { cg := testsutil.CreateContainerGroupObj(podName, podNamespace, "Succeeded", - testsutil.CreateACIContainersListObj("Running", "Initializing", testsutil.CgCreationTime.Add(time.Second*2), testsutil.CgCreationTime.Add(time.Second*3), false, false, false), "Succeeded") + testsutil.CreateACIContainersListObj(runningState, "Initializing", testsutil.CgCreationTime.Add(time.Second*2), testsutil.CgCreationTime.Add(time.Second*3), false, false, false), "Succeeded") cgID = *cg.ID return cg, nil } diff --git a/pkg/provider/aci_volumes_test.go b/pkg/provider/aci_volumes_test.go index 6743890b..335c2277 100644 --- a/pkg/provider/aci_volumes_test.go +++ b/pkg/provider/aci_volumes_test.go @@ -13,7 +13,6 @@ import ( azaciv2 "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/containerinstance/armcontainerinstance/v2" "github.com/golang/mock/gomock" "github.com/google/uuid" - "github.com/virtual-kubelet/azure-aci/pkg/client" "github.com/virtual-kubelet/azure-aci/pkg/featureflag" testsutil "github.com/virtual-kubelet/azure-aci/pkg/tests" "github.com/virtual-kubelet/node-cli/manager" @@ -60,11 +59,11 @@ func TestCreatedPodWithAzureFilesVolume(t *testing.T) { initEnabled := provider.enabledFeatures.IsEnabled(context.TODO(), featureflag.InitContainerFeature) - aciMocks.MockCreateContainerGroup = func(ctx context.Context, resourceGroup, podNS, podName string, cg *client.ContainerGroupWrapper) error { - containers := cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Properties.Containers + aciMocks.MockCreateContainerGroup = func(ctx context.Context, resourceGroup, podNS, podName string, cg *azaciv2.ContainerGroup) error { + containers := cg.Properties.Containers // Check only if init container feature is enabled if initEnabled { - initContainers := cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Properties.InitContainers + initContainers := cg.Properties.InitContainers assert.Check(t, initContainers[0].Properties.VolumeMounts != nil, "Volume mount should be present") assert.Check(t, initContainers[0].Properties.EnvironmentVariables != nil, "Volume mount should be present") assert.Check(t, initContainers[0].Properties.Command != nil, "Command mount should be present") @@ -76,11 +75,11 @@ func TestCreatedPodWithAzureFilesVolume(t *testing.T) { assert.Check(t, containers != nil, "Containers should not be nil") assert.Check(t, is.Equal(1, len(containers)), "1 Container is expected") assert.Check(t, is.Equal("nginx", *(containers)[0].Name), "Container nginx is expected") - assert.Check(t, is.Equal(3, len(cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Properties.Volumes)), "volume count not match") - assert.Check(t, is.Equal(azureFileVolumeName1, *(cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Properties.Volumes)[1].Name), "volume name is not matched") - assert.Check(t, is.Equal(fakeShareName1, *(cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Properties.Volumes)[1].AzureFile.ShareName), "volume share name is not matched") - assert.Check(t, is.Equal(azureFileVolumeName2, *(cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Properties.Volumes)[2].Name), "volume name is not matched") - assert.Check(t, is.Equal(fakeShareName2, *(cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Properties.Volumes)[2].AzureFile.ShareName), "volume share name is not matched") + assert.Check(t, is.Equal(3, len(cg.Properties.Volumes)), "volume count not match") + assert.Check(t, is.Equal(azureFileVolumeName1, *(cg.Properties.Volumes)[1].Name), "volume name is not matched") + assert.Check(t, is.Equal(fakeShareName1, *(cg.Properties.Volumes)[1].AzureFile.ShareName), "volume share name is not matched") + assert.Check(t, is.Equal(azureFileVolumeName2, *(cg.Properties.Volumes)[2].Name), "volume name is not matched") + assert.Check(t, is.Equal(fakeShareName2, *(cg.Properties.Volumes)[2].AzureFile.ShareName), "volume share name is not matched") return nil } @@ -249,10 +248,10 @@ func TestCreatePodWithProjectedVolume(t *testing.T) { aciMocks := createNewACIMock() encodedSecretVal := base64.StdEncoding.EncodeToString([]byte("fake-ca-data")) - aciMocks.MockCreateContainerGroup = func(ctx context.Context, resourceGroup, podNS, podName string, cg *client.ContainerGroupWrapper) error { - containers := cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Properties.Containers - volumes := cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Properties.Volumes - certVal := cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Properties.Volumes[2].Secret["ca.crt"] + aciMocks.MockCreateContainerGroup = func(ctx context.Context, resourceGroup, podNS, podName string, cg *azaciv2.ContainerGroup) error { + containers := cg.Properties.Containers + volumes := cg.Properties.Volumes + certVal := cg.Properties.Volumes[2].Secret["ca.crt"] assert.Check(t, cg != nil, "Container group is nil") assert.Check(t, containers != nil, "Containers should not be nil") assert.Check(t, is.Equal(1, len(containers)), "1 Container is expected") @@ -340,13 +339,13 @@ func TestCreatePodWithCSIVolume(t *testing.T) { azureFileVolumeName := "azure" aciMocks := createNewACIMock() - aciMocks.MockCreateContainerGroup = func(ctx context.Context, resourceGroup, podNS, podName string, cg *client.ContainerGroupWrapper) error { - containers := cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Properties.Containers + aciMocks.MockCreateContainerGroup = func(ctx context.Context, resourceGroup, podNS, podName string, cg *azaciv2.ContainerGroup) error { + containers := cg.Properties.Containers assert.Check(t, cg != nil, "Container group is nil") assert.Check(t, containers != nil, "Containers should not be nil") assert.Check(t, is.Equal(1, len(containers)), "1 Container is expected") assert.Check(t, is.Equal("nginx", *(containers[0]).Name), "Container nginx is expected") - assert.Check(t, is.Equal(2, len(cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Properties.Volumes)), "volume count not match") + assert.Check(t, is.Equal(2, len(cg.Properties.Volumes)), "volume count not match") return nil } diff --git a/pkg/provider/mock_aci_test.go b/pkg/provider/mock_aci_test.go index ae1e530b..afa60500 100644 --- a/pkg/provider/mock_aci_test.go +++ b/pkg/provider/mock_aci_test.go @@ -4,11 +4,10 @@ import ( "context" azaciv2 "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/containerinstance/armcontainerinstance/v2" - "github.com/virtual-kubelet/azure-aci/pkg/client" "github.com/virtual-kubelet/virtual-kubelet/node/api" ) -type CreateContainerGroupFunc func(ctx context.Context, resourceGroup, podNS, podName string, cg *client.ContainerGroupWrapper) error +type CreateContainerGroupFunc func(ctx context.Context, resourceGroup, podNS, podName string, cg *azaciv2.ContainerGroup) error type GetContainerGroupInfoFunc func(ctx context.Context, resourceGroup, namespace, name, nodeName string) (*azaciv2.ContainerGroup, error) type GetContainerGroupListFunc func(ctx context.Context, resourceGroup string) ([]*azaciv2.ContainerGroup, error) type ListCapabilitiesFunc func(ctx context.Context, region string) ([]*azaciv2.Capabilities, error) @@ -57,7 +56,7 @@ func (m *MockACIProvider) GetContainerGroupInfo(ctx context.Context, resourceGro return nil, nil } -func (m *MockACIProvider) CreateContainerGroup(ctx context.Context, resourceGroup, podNS, podName string, cg *client.ContainerGroupWrapper) error { +func (m *MockACIProvider) CreateContainerGroup(ctx context.Context, resourceGroup, podNS, podName string, cg *azaciv2.ContainerGroup) error { if m.MockCreateContainerGroup != nil { return m.MockCreateContainerGroup(ctx, resourceGroup, podNS, podName, cg) }