From e979e998eba0e09eaf31bfc209164ed7afc15e9b Mon Sep 17 00:00:00 2001 From: t-ysalazar Date: Tue, 7 Jun 2022 10:58:05 -0700 Subject: [PATCH 01/46] ignore helm init and pod specs example --- .gitignore | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.gitignore b/.gitignore index b66fb05f..94fc75f5 100644 --- a/.gitignore +++ b/.gitignore @@ -4,6 +4,10 @@ *.dll *.so *.dylib +*.bash + +helm-start.sh +podspcs.yaml # Test binary, build with `go test -c` *.test From 7fc6c1b7274924bbd7f7df4daa3a378d72c05f83 Mon Sep 17 00:00:00 2001 From: t-ysalazar Date: Tue, 7 Jun 2022 11:49:32 -0700 Subject: [PATCH 02/46] add initContainers property --- client/aci/types.go | 1 + 1 file changed, 1 insertion(+) diff --git a/client/aci/types.go b/client/aci/types.go index 0d51cb50..54a71509 100644 --- a/client/aci/types.go +++ b/client/aci/types.go @@ -87,6 +87,7 @@ type ContainerGroup struct { type ContainerGroupProperties struct { ProvisioningState string `json:"provisioningState,omitempty"` Containers []Container `json:"containers,omitempty"` + InitContainers []InitContainerDefinition `json:"initContainers,omitempty"` ImageRegistryCredentials []ImageRegistryCredential `json:"imageRegistryCredentials,omitempty"` RestartPolicy ContainerGroupRestartPolicy `json:"restartPolicy,omitempty"` IPAddress *IPAddress `json:"ipAddress,omitempty"` From 16c676bf16baa2414394971fe4731fcdbdf121f0 Mon Sep 17 00:00:00 2001 From: t-ysalazar Date: Tue, 7 Jun 2022 13:21:35 -0700 Subject: [PATCH 03/46] declaration of getInitContainers --- provider/aci.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/provider/aci.go b/provider/aci.go index 582833ef..a8bf9118 100644 --- a/provider/aci.go +++ b/provider/aci.go @@ -667,6 +667,11 @@ func (p *ACIProvider) CreatePod(ctx context.Context, pod *v1.Pod) error { containerGroup.RestartPolicy = aci.ContainerGroupRestartPolicy(pod.Spec.RestartPolicy) containerGroup.ContainerGroupProperties.OsType = aci.OperatingSystemTypes(p.operatingSystem) + // get initContainers + initContainers, err := p.getInitContainers(pod) + if err != nil { + return err + } // get containers containers, err := p.getContainers(pod) if err != nil { @@ -683,6 +688,7 @@ func (p *ACIProvider) CreatePod(ctx context.Context, pod *v1.Pod) error { return err } // assign all the things + containerGroup.ContainerGroupProperties.InitContainers = initContainers containerGroup.ContainerGroupProperties.Containers = containers containerGroup.ContainerGroupProperties.Volumes = volumes containerGroup.ContainerGroupProperties.ImageRegistryCredentials = creds @@ -1443,6 +1449,10 @@ func readDockerConfigJSONSecret(secret *v1.Secret, ips []aci.ImageRegistryCreden return ips, err } +func (p *ACIProvider) getInitContainers(pod *v1.Pod) ([]aci.InitContainerDefinition, error) { + return initContainers, nil +} + func (p *ACIProvider) getContainers(pod *v1.Pod) ([]aci.Container, error) { containers := make([]aci.Container, 0, len(pod.Spec.Containers)) for _, container := range pod.Spec.Containers { From c991e03cca6e89b012dea3636438ca134bb0e9a0 Mon Sep 17 00:00:00 2001 From: t-ysalazar Date: Tue, 7 Jun 2022 14:58:04 -0700 Subject: [PATCH 04/46] getInitContainers --- provider/aci.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/provider/aci.go b/provider/aci.go index a8bf9118..f9948c69 100644 --- a/provider/aci.go +++ b/provider/aci.go @@ -1450,6 +1450,10 @@ func readDockerConfigJSONSecret(secret *v1.Secret, ips []aci.ImageRegistryCreden } func (p *ACIProvider) getInitContainers(pod *v1.Pod) ([]aci.InitContainerDefinition, error) { + initContainers := make([]aci.InitContainerDefinition, 0, len(pod.Spec.InitContainers)) + for _, initContainer := range pod.Spec.InitContainers { + + } return initContainers, nil } From cbe2be4435b751fe721f91a0b3cfacd3f533ccba Mon Sep 17 00:00:00 2001 From: t-ysalazar Date: Tue, 7 Jun 2022 15:17:33 -0700 Subject: [PATCH 05/46] factorize getVolumeMounts() from getContainers() --- provider/aci.go | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/provider/aci.go b/provider/aci.go index f9948c69..93ff01e3 100644 --- a/provider/aci.go +++ b/provider/aci.go @@ -1449,6 +1449,18 @@ func readDockerConfigJSONSecret(secret *v1.Secret, ips []aci.ImageRegistryCreden return ips, err } +func (p *ACIProvider) getVolumeMounts(container) ([]aci.VolumeMount) { + volumeMounts = make([]aci.VolumeMount, 0, len(container.VolumeMounts)) + for _, v := range container.VolumeMounts { + volumeMounts= append(volumeMounts, aci.VolumeMount{ + Name: v.Name, + MountPath: v.MountPath, + ReadOnly: v.ReadOnly, + }) + } + return volumeMounts +} + func (p *ACIProvider) getInitContainers(pod *v1.Pod) ([]aci.InitContainerDefinition, error) { initContainers := make([]aci.InitContainerDefinition, 0, len(pod.Spec.InitContainers)) for _, initContainer := range pod.Spec.InitContainers { @@ -1480,14 +1492,7 @@ func (p *ACIProvider) getContainers(pod *v1.Pod) ([]aci.Container, error) { }) } - c.VolumeMounts = make([]aci.VolumeMount, 0, len(container.VolumeMounts)) - for _, v := range container.VolumeMounts { - c.VolumeMounts = append(c.VolumeMounts, aci.VolumeMount{ - Name: v.Name, - MountPath: v.MountPath, - ReadOnly: v.ReadOnly, - }) - } + c.VolumeMounts = getVolumeMounts(container) c.EnvironmentVariables = make([]aci.EnvironmentVariable, 0, len(container.Env)) for _, e := range container.Env { From ff1a32f9f419cd424ccdce695ee54914282ef5a1 Mon Sep 17 00:00:00 2001 From: t-ysalazar Date: Tue, 7 Jun 2022 15:58:28 -0700 Subject: [PATCH 06/46] fix type errors --- client/aci/types.go | 2 +- provider/aci.go | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/client/aci/types.go b/client/aci/types.go index 54a71509..4b58e733 100644 --- a/client/aci/types.go +++ b/client/aci/types.go @@ -87,7 +87,7 @@ type ContainerGroup struct { type ContainerGroupProperties struct { ProvisioningState string `json:"provisioningState,omitempty"` Containers []Container `json:"containers,omitempty"` - InitContainers []InitContainerDefinition `json:"initContainers,omitempty"` + InitContainers []Container `json:"initContainers,omitempty"` ImageRegistryCredentials []ImageRegistryCredential `json:"imageRegistryCredentials,omitempty"` RestartPolicy ContainerGroupRestartPolicy `json:"restartPolicy,omitempty"` IPAddress *IPAddress `json:"ipAddress,omitempty"` diff --git a/provider/aci.go b/provider/aci.go index 93ff01e3..df78f5bc 100644 --- a/provider/aci.go +++ b/provider/aci.go @@ -1449,10 +1449,10 @@ func readDockerConfigJSONSecret(secret *v1.Secret, ips []aci.ImageRegistryCreden return ips, err } -func (p *ACIProvider) getVolumeMounts(container) ([]aci.VolumeMount) { - volumeMounts = make([]aci.VolumeMount, 0, len(container.VolumeMounts)) +func (p *ACIProvider) getVolumeMounts(container *v1.Container) []aci.VolumeMount { + volumeMounts := make([]aci.VolumeMount, 0, len(container.VolumeMounts)) for _, v := range container.VolumeMounts { - volumeMounts= append(volumeMounts, aci.VolumeMount{ + volumeMounts = append(volumeMounts, aci.VolumeMount{ Name: v.Name, MountPath: v.MountPath, ReadOnly: v.ReadOnly, @@ -1461,11 +1461,11 @@ func (p *ACIProvider) getVolumeMounts(container) ([]aci.VolumeMount) { return volumeMounts } -func (p *ACIProvider) getInitContainers(pod *v1.Pod) ([]aci.InitContainerDefinition, error) { - initContainers := make([]aci.InitContainerDefinition, 0, len(pod.Spec.InitContainers)) - for _, initContainer := range pod.Spec.InitContainers { +func (p *ACIProvider) getInitContainers(pod *v1.Pod) ([]aci.Container, error) { + initContainers := make([]aci.Container, 0, len(pod.Spec.InitContainers)) + /*for _, initContainer := range pod.Spec.InitContainers { - } + }*/ return initContainers, nil } @@ -1492,7 +1492,7 @@ func (p *ACIProvider) getContainers(pod *v1.Pod) ([]aci.Container, error) { }) } - c.VolumeMounts = getVolumeMounts(container) + c.VolumeMounts = p.getVolumeMounts(&container) c.EnvironmentVariables = make([]aci.EnvironmentVariable, 0, len(container.Env)) for _, e := range container.Env { From 1b051a5f960e5efa990bb3c054b220739fe3cbc4 Mon Sep 17 00:00:00 2001 From: t-ysalazar Date: Tue, 7 Jun 2022 16:13:20 -0700 Subject: [PATCH 07/46] factorize getEnvironmentVariables --- provider/aci.go | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/provider/aci.go b/provider/aci.go index df78f5bc..08b185a8 100644 --- a/provider/aci.go +++ b/provider/aci.go @@ -1461,6 +1461,17 @@ func (p *ACIProvider) getVolumeMounts(container *v1.Container) []aci.VolumeMount return volumeMounts } +func (p *ACIProvider) getEnvironmentVariables(container *v1.Container) []aci.EnvironmentVariable { + environmentVariable := make([]aci.EnvironmentVariable, 0, len(container.Env)) + for _, e := range container.Env { + if e.Value != "" { + envVar := getACIEnvVar(e) + environmentVariable = append(environmentVariable, envVar) + } + } + return environmentVariable +} + func (p *ACIProvider) getInitContainers(pod *v1.Pod) ([]aci.Container, error) { initContainers := make([]aci.Container, 0, len(pod.Spec.InitContainers)) /*for _, initContainer := range pod.Spec.InitContainers { @@ -1494,13 +1505,7 @@ func (p *ACIProvider) getContainers(pod *v1.Pod) ([]aci.Container, error) { c.VolumeMounts = p.getVolumeMounts(&container) - c.EnvironmentVariables = make([]aci.EnvironmentVariable, 0, len(container.Env)) - for _, e := range container.Env { - if e.Value != "" { - envVar := getACIEnvVar(e) - c.EnvironmentVariables = append(c.EnvironmentVariables, envVar) - } - } + c.EnvironmentVariables = p.getEnvironmentVariables(&container) // NOTE(robbiezhang): ACI CPU request must be times of 10m cpuRequest := 1.00 From b9671717526ace8f2ee04fbaa97bb14e341d8077 Mon Sep 17 00:00:00 2001 From: t-ysalazar Date: Tue, 7 Jun 2022 17:23:36 -0700 Subject: [PATCH 08/46] basics of init containers on getInitContainer --- provider/aci.go | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/provider/aci.go b/provider/aci.go index 08b185a8..48d4c98c 100644 --- a/provider/aci.go +++ b/provider/aci.go @@ -1474,9 +1474,21 @@ func (p *ACIProvider) getEnvironmentVariables(container *v1.Container) []aci.Env func (p *ACIProvider) getInitContainers(pod *v1.Pod) ([]aci.Container, error) { initContainers := make([]aci.Container, 0, len(pod.Spec.InitContainers)) - /*for _, initContainer := range pod.Spec.InitContainers { + for _, initContainer := range pod.Spec.InitContainers { + c := aci.Container{ + Name: initContainer.Name, + ContainerProperties: aci.ContainerProperties{ + Image: initContainer.Image, + Command: append(initContainer.Command, initContainer.Args...), + }, + } - }*/ + c.VolumeMounts = p.getVolumeMounts(&initContainer) + + c.EnvironmentVariables = p.getEnvironmentVariables(&initContainer) + + initContainers = append(initContainers, c) + } return initContainers, nil } From 0ed160dfe5ba928c20b1ed45fbb574565234dc89 Mon Sep 17 00:00:00 2001 From: t-ysalazar Date: Wed, 8 Jun 2022 10:46:02 -0700 Subject: [PATCH 09/46] getBasicContainer --- client/aci/types.go | 10 +++++----- provider/aci.go | 28 +++++++++++++++++----------- 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/client/aci/types.go b/client/aci/types.go index 4b58e733..36418dd3 100644 --- a/client/aci/types.go +++ b/client/aci/types.go @@ -87,7 +87,7 @@ type ContainerGroup struct { type ContainerGroupProperties struct { ProvisioningState string `json:"provisioningState,omitempty"` Containers []Container `json:"containers,omitempty"` - InitContainers []Container `json:"initContainers,omitempty"` + InitContainers []Container `json:"initContainers,omitempty"` ImageRegistryCredentials []ImageRegistryCredential `json:"imageRegistryCredentials,omitempty"` RestartPolicy ContainerGroupRestartPolicy `json:"restartPolicy,omitempty"` IPAddress *IPAddress `json:"ipAddress,omitempty"` @@ -95,7 +95,7 @@ type ContainerGroupProperties struct { Volumes []Volume `json:"volumes,omitempty"` InstanceView ContainerGroupPropertiesInstanceView `json:"instanceView,omitempty"` Diagnostics *ContainerGroupDiagnostics `json:"diagnostics,omitempty"` - SubnetIds []*SubnetIdDefinition `json:"subnetIds,omitempty"` + SubnetIds []*SubnetIdDefinition `json:"subnetIds,omitempty"` Extensions []*Extension `json:"extensions,omitempty"` DNSConfig *DNSConfig `json:"dnsConfig,omitempty"` } @@ -106,7 +106,7 @@ type ContainerGroupPropertiesInstanceView struct { State string `json:"state,omitempty"` } -// SubnetIdDefinition is the subnet ID, the format should be +// SubnetIdDefinition is the subnet ID, the format should be // /subscriptions/{subscriptionID}/resourceGroups/{ResourceGroup}/providers/Microsoft.Network/virtualNetworks/{VNET}/subnets/{Subnet} type SubnetIdDefinition struct { ID string `json:"id,omitempty"` @@ -243,7 +243,7 @@ type GPUSKU string const ( // K80 specifies the K80 GPU SKU - K80 GPUSKU = "K80" + K80 GPUSKU = "K80" // P100 specifies the P100 GPU SKU P100 GPUSKU = "P100" // V100 specifies the V100 GPU SKU @@ -461,7 +461,7 @@ type ExtensionType string // Supported extension types const ( - ExtensionTypeKubeProxy ExtensionType = "kube-proxy" + ExtensionTypeKubeProxy ExtensionType = "kube-proxy" ExtensionTypeRealtimeMetrics ExtensionType = "realtime-metrics" ) diff --git a/provider/aci.go b/provider/aci.go index 48d4c98c..91a4d2a3 100644 --- a/provider/aci.go +++ b/provider/aci.go @@ -1449,6 +1449,19 @@ func readDockerConfigJSONSecret(secret *v1.Secret, ips []aci.ImageRegistryCreden return ips, err } +func (p *ACIProvider) getBasicContainer(container *v1.Container) (aci.Container, error) { + if len(container.Command) == 0 && len(container.Args) > 0 { + return aci.Container{}, errdefs.InvalidInput("ACI does not support providing args without specifying the command. Please supply both command and args to the pod spec.") + } + return aci.Container{ + Name: container.Name, + ContainerProperties: aci.ContainerProperties{ + Image: container.Image, + Command: append(container.Command, container.Args...), + }, + }, nil +} + func (p *ACIProvider) getVolumeMounts(container *v1.Container) []aci.VolumeMount { volumeMounts := make([]aci.VolumeMount, 0, len(container.VolumeMounts)) for _, v := range container.VolumeMounts { @@ -1495,19 +1508,12 @@ func (p *ACIProvider) getInitContainers(pod *v1.Pod) ([]aci.Container, error) { func (p *ACIProvider) getContainers(pod *v1.Pod) ([]aci.Container, error) { containers := make([]aci.Container, 0, len(pod.Spec.Containers)) for _, container := range pod.Spec.Containers { - - if len(container.Command) == 0 && len(container.Args) > 0 { - return nil, errdefs.InvalidInput("ACI does not support providing args without specifying the command. Please supply both command and args to the pod spec.") - } - c := aci.Container{ - Name: container.Name, - ContainerProperties: aci.ContainerProperties{ - Image: container.Image, - Command: append(container.Command, container.Args...), - Ports: make([]aci.ContainerPort, 0, len(container.Ports)), - }, + c, err := p.getBasicContainer(&container) + if err != nil { + return nil, err } + c.ContainerProperties.Ports = make([]aci.ContainerPort, 0, len(container.Ports)) for _, p := range container.Ports { c.Ports = append(c.Ports, aci.ContainerPort{ Port: p.ContainerPort, From 179f9ceb25a6f256c12c029c6b670e6c2071ea76 Mon Sep 17 00:00:00 2001 From: t-ysalazar Date: Wed, 8 Jun 2022 10:47:42 -0700 Subject: [PATCH 10/46] getBasicContainer on initContainer --- provider/aci.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/provider/aci.go b/provider/aci.go index 91a4d2a3..e1ea0c5f 100644 --- a/provider/aci.go +++ b/provider/aci.go @@ -1488,12 +1488,9 @@ func (p *ACIProvider) getEnvironmentVariables(container *v1.Container) []aci.Env func (p *ACIProvider) getInitContainers(pod *v1.Pod) ([]aci.Container, error) { initContainers := make([]aci.Container, 0, len(pod.Spec.InitContainers)) for _, initContainer := range pod.Spec.InitContainers { - c := aci.Container{ - Name: initContainer.Name, - ContainerProperties: aci.ContainerProperties{ - Image: initContainer.Image, - Command: append(initContainer.Command, initContainer.Args...), - }, + c, err := p.getBasicContainer(&initContainer) + if err != nil { + return nil, err } c.VolumeMounts = p.getVolumeMounts(&initContainer) From c5fba5289c0982d2a6a225ae3163b82f881fe8be Mon Sep 17 00:00:00 2001 From: t-ysalazar Date: Wed, 8 Jun 2022 17:00:14 -0700 Subject: [PATCH 11/46] implementation of InitContainerDefinition type --- client/aci/types.go | 17 ++++++++++++++++- provider/aci.go | 16 +++++++++++++--- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/client/aci/types.go b/client/aci/types.go index 36418dd3..72c917b5 100644 --- a/client/aci/types.go +++ b/client/aci/types.go @@ -72,6 +72,12 @@ type Container struct { ContainerProperties `json:"properties,omitempty"` } +// InitContainerDefinition is a initContainer instance. +type InitContainerDefinition struct { + Name string `json:"name,omitempty"` + InitContainerProperties `json:"properties,omitempty"` +} + // ContainerGroup is a container group. type ContainerGroup struct { api.ResponseMetadata `json:"-"` @@ -87,7 +93,7 @@ type ContainerGroup struct { type ContainerGroupProperties struct { ProvisioningState string `json:"provisioningState,omitempty"` Containers []Container `json:"containers,omitempty"` - InitContainers []Container `json:"initContainers,omitempty"` + InitContainers []InitContainerDefinition `json:"initContainers,omitempty"` ImageRegistryCredentials []ImageRegistryCredential `json:"imageRegistryCredentials,omitempty"` RestartPolicy ContainerGroupRestartPolicy `json:"restartPolicy,omitempty"` IPAddress *IPAddress `json:"ipAddress,omitempty"` @@ -138,6 +144,15 @@ type ContainerProperties struct { ReadinessProbe *ContainerProbe `json:"readinessProbe,omitempty"` } +// ContainerProperties is the container instance properties. +type InitContainerProperties struct { + Image string `json:"image,omitempty"` + Command []string `json:"command,omitempty"` + EnvironmentVariables []EnvironmentVariable `json:"environmentVariables,omitempty"` + InstanceView ContainerPropertiesInstanceView `json:"instanceView,omitempty"` + VolumeMounts []VolumeMount `json:"volumeMounts,omitempty"` +} + // ContainerPropertiesInstanceView is the instance view of the container instance. Only valid in response. type ContainerPropertiesInstanceView struct { RestartCount int32 `json:"restartCount,omitempty"` diff --git a/provider/aci.go b/provider/aci.go index e1ea0c5f..715db74f 100644 --- a/provider/aci.go +++ b/provider/aci.go @@ -1485,8 +1485,8 @@ func (p *ACIProvider) getEnvironmentVariables(container *v1.Container) []aci.Env return environmentVariable } -func (p *ACIProvider) getInitContainers(pod *v1.Pod) ([]aci.Container, error) { - initContainers := make([]aci.Container, 0, len(pod.Spec.InitContainers)) +func (p *ACIProvider) getInitContainers(pod *v1.Pod) ([]aci.InitContainerDefinition, error) { + initContainers := make([]aci.InitContainerDefinition, 0, len(pod.Spec.InitContainers)) for _, initContainer := range pod.Spec.InitContainers { c, err := p.getBasicContainer(&initContainer) if err != nil { @@ -1497,7 +1497,17 @@ func (p *ACIProvider) getInitContainers(pod *v1.Pod) ([]aci.Container, error) { c.EnvironmentVariables = p.getEnvironmentVariables(&initContainer) - initContainers = append(initContainers, c) + //for reuse code, first declare a container and then translate to InitContainerDefinition + newInitContainer := aci.InitContainerDefinition{ + Name: c.Name, + InitContainerProperties: aci.InitContainerProperties{ + Image: c.Image, + Command: c.Command, + EnvironmentVariables: c.EnvironmentVariables, + VolumeMounts: c.VolumeMounts, + }, + } + initContainers = append(initContainers, newInitContainer) } return initContainers, nil } From 4ac821abfd819b9173b46a272e0126bb770698bd Mon Sep 17 00:00:00 2001 From: t-ysalazar Date: Wed, 8 Jun 2022 18:54:24 -0700 Subject: [PATCH 12/46] factorize verifyContainer from getContainer --- client/aci/types.go | 2 +- provider/aci.go | 50 ++++++++++++++++++++++----------------------- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/client/aci/types.go b/client/aci/types.go index 72c917b5..b42c6510 100644 --- a/client/aci/types.go +++ b/client/aci/types.go @@ -144,7 +144,7 @@ type ContainerProperties struct { ReadinessProbe *ContainerProbe `json:"readinessProbe,omitempty"` } -// ContainerProperties is the container instance properties. +// InitContainerProperties is the initContainer instance properties. type InitContainerProperties struct { Image string `json:"image,omitempty"` Command []string `json:"command,omitempty"` diff --git a/provider/aci.go b/provider/aci.go index 715db74f..0545be4f 100644 --- a/provider/aci.go +++ b/provider/aci.go @@ -1449,17 +1449,15 @@ func readDockerConfigJSONSecret(secret *v1.Secret, ips []aci.ImageRegistryCreden return ips, err } -func (p *ACIProvider) getBasicContainer(container *v1.Container) (aci.Container, error) { +func (p *ACIProvider) verifyContainer(container *v1.Container) error { if len(container.Command) == 0 && len(container.Args) > 0 { - return aci.Container{}, errdefs.InvalidInput("ACI does not support providing args without specifying the command. Please supply both command and args to the pod spec.") + return errdefs.InvalidInput("ACI does not support providing args without specifying the command. Please supply both command and args to the pod spec.") } - return aci.Container{ - Name: container.Name, - ContainerProperties: aci.ContainerProperties{ - Image: container.Image, - Command: append(container.Command, container.Args...), - }, - }, nil + return nil +} + +func (p *ACIProvider) getCommand(container *v1.Container) []string { + return append(container.Command, container.Args...) } func (p *ACIProvider) getVolumeMounts(container *v1.Container) []aci.VolumeMount { @@ -1488,25 +1486,21 @@ func (p *ACIProvider) getEnvironmentVariables(container *v1.Container) []aci.Env func (p *ACIProvider) getInitContainers(pod *v1.Pod) ([]aci.InitContainerDefinition, error) { initContainers := make([]aci.InitContainerDefinition, 0, len(pod.Spec.InitContainers)) for _, initContainer := range pod.Spec.InitContainers { - c, err := p.getBasicContainer(&initContainer) + err := p.verifyContainer(&initContainer) if err != nil { return nil, err } - c.VolumeMounts = p.getVolumeMounts(&initContainer) - - c.EnvironmentVariables = p.getEnvironmentVariables(&initContainer) - - //for reuse code, first declare a container and then translate to InitContainerDefinition newInitContainer := aci.InitContainerDefinition{ - Name: c.Name, - InitContainerProperties: aci.InitContainerProperties{ - Image: c.Image, - Command: c.Command, - EnvironmentVariables: c.EnvironmentVariables, - VolumeMounts: c.VolumeMounts, - }, + Name: initContainer.Name, } + + newInitContainer.Image = initContainer.Image + newInitContainer.Command = p.getCommand(&initContainer) + + newInitContainer.VolumeMounts = p.getVolumeMounts(&initContainer) + newInitContainer.EnvironmentVariables = p.getEnvironmentVariables(&initContainer) + initContainers = append(initContainers, newInitContainer) } return initContainers, nil @@ -1515,12 +1509,19 @@ func (p *ACIProvider) getInitContainers(pod *v1.Pod) ([]aci.InitContainerDefinit func (p *ACIProvider) getContainers(pod *v1.Pod) ([]aci.Container, error) { containers := make([]aci.Container, 0, len(pod.Spec.Containers)) for _, container := range pod.Spec.Containers { - c, err := p.getBasicContainer(&container) + err := p.verifyContainer(&container) if err != nil { return nil, err } - c.ContainerProperties.Ports = make([]aci.ContainerPort, 0, len(container.Ports)) + c := aci.Container{ + Name: container.Name, + } + + c.Image = container.Image + c.Command = p.getCommand(&container) + + c.Ports = make([]aci.ContainerPort, 0, len(container.Ports)) for _, p := range container.Ports { c.Ports = append(c.Ports, aci.ContainerPort{ Port: p.ContainerPort, @@ -1529,7 +1530,6 @@ func (p *ACIProvider) getContainers(pod *v1.Pod) ([]aci.Container, error) { } c.VolumeMounts = p.getVolumeMounts(&container) - c.EnvironmentVariables = p.getEnvironmentVariables(&container) // NOTE(robbiezhang): ACI CPU request must be times of 10m From 4a63bbf53e30368eeb7a759045a8c34ed22c365e Mon Sep 17 00:00:00 2001 From: t-ysalazar Date: Thu, 9 Jun 2022 10:24:38 -0700 Subject: [PATCH 13/46] throw erros with unsupported properties in ACI initContainers --- provider/aci.go | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/provider/aci.go b/provider/aci.go index 0545be4f..88b4d839 100644 --- a/provider/aci.go +++ b/provider/aci.go @@ -1491,10 +1491,25 @@ func (p *ACIProvider) getInitContainers(pod *v1.Pod) ([]aci.InitContainerDefinit return nil, err } + if initContainer.Ports != nil { + return nil, errdefs.InvalidInput("ACI initContainers does not support ports.") + } + if initContainer.Resources.Requests != nil { + return nil, errdefs.InvalidInput("ACI initContainers does not support resources requests.") + } + if initContainer.Resources.Limits != nil { + return nil, errdefs.InvalidInput("ACI initContainers does not support resources limits.") + } + if initContainer.LivenessProbe != nil { + return nil, errdefs.InvalidInput("ACI initContainers does not support livenessProbe.") + } + if initContainer.ReadinessProbe != nil { + return nil, errdefs.InvalidInput("ACI initContainers does not support readinessProbe.") + } + newInitContainer := aci.InitContainerDefinition{ Name: initContainer.Name, } - newInitContainer.Image = initContainer.Image newInitContainer.Command = p.getCommand(&initContainer) @@ -1517,7 +1532,6 @@ func (p *ACIProvider) getContainers(pod *v1.Pod) ([]aci.Container, error) { c := aci.Container{ Name: container.Name, } - c.Image = container.Image c.Command = p.getCommand(&container) From c98c7c4cc472afbdf0194a4c4cf08aaa5a9f559f Mon Sep 17 00:00:00 2001 From: t-ysalazar Date: Thu, 9 Jun 2022 10:52:57 -0700 Subject: [PATCH 14/46] fix .gitignore for PR --- .gitignore | 1 - 1 file changed, 1 deletion(-) diff --git a/.gitignore b/.gitignore index 94fc75f5..729a9fbc 100644 --- a/.gitignore +++ b/.gitignore @@ -4,7 +4,6 @@ *.dll *.so *.dylib -*.bash helm-start.sh podspcs.yaml From 3d4bd30336f08ec6ef615e6899418cbae49b88bd Mon Sep 17 00:00:00 2001 From: t-ysalazar Date: Thu, 9 Jun 2022 14:19:18 -0700 Subject: [PATCH 15/46] unit test for InitContainers --- provider/aci_test.go | 52 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/provider/aci_test.go b/provider/aci_test.go index 0443c85f..a6a46168 100644 --- a/provider/aci_test.go +++ b/provider/aci_test.go @@ -1399,3 +1399,55 @@ func TestCreatePodWithCSIVolume(t *testing.T) { }) } } + +func TestCreatePodWithInitContainer(t *testing.T) { + _, aciServerMocker, provider, err := prepareMocks() + + if err != nil { + t.Fatal("Unable to prepare the mocks", err) + } + + podName := "pod-" + uuid.New().String() + podNamespace := "ns-" + uuid.New().String() + + aciServerMocker.OnCreate = func(subscription, resourceGroup, containerGroup string, cg *aci.ContainerGroup) (int, interface{}) { + assert.Check(t, is.Equal(fakeSubscription, subscription), "Subscription doesn't match") + assert.Check(t, is.Equal(fakeResourceGroup, resourceGroup), "Resource group doesn't match") + assert.Check(t, cg != nil, "Container group is nil") + assert.Check(t, cg.ContainerGroupProperties.Containers != nil, "Containers should not be nil") + assert.Check(t, cg.ContainerGroupProperties.InitContainers != nil, "InitContainer should not be nil") + assert.Check(t, is.Equal(1, len(cg.ContainerGroupProperties.Containers)), "1 Container is expected") + assert.Check(t, is.Equal(2, len(cg.ContainerGroupProperties.InitContainers)), "2 InitContainer are expected") + assert.Check(t, is.Equal("nginx", cg.ContainerGroupProperties.Containers[0].Name), "Container nginx is expected") + + return http.StatusOK, cg + } + + pod := &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: podName, + Namespace: podNamespace, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "nginx", + }, + }, + InitContainers: []v1.Container{ + { + Name: "initContainer 01", + Image: "alpine", + }, + { + Name: "initContainer 02", + Image: "alpine", + }, + }, + }, + } + + if err := provider.CreatePod(context.Background(), pod); err != nil { + t.Fatal("Failed to create pod", err) + } +} From db4eef2f1d81fb5c33c0484b268df63e6e0ef0ed Mon Sep 17 00:00:00 2001 From: t-ysalazar Date: Fri, 10 Jun 2022 10:07:57 -0700 Subject: [PATCH 16/46] revert gitignore changes --- .gitignore | 3 --- 1 file changed, 3 deletions(-) diff --git a/.gitignore b/.gitignore index 729a9fbc..b66fb05f 100644 --- a/.gitignore +++ b/.gitignore @@ -5,9 +5,6 @@ *.so *.dylib -helm-start.sh -podspcs.yaml - # Test binary, build with `go test -c` *.test From 98686e21e138694fad39146c753172e9e8a56fe1 Mon Sep 17 00:00:00 2001 From: t-ysalazar Date: Fri, 10 Jun 2022 10:30:41 -0700 Subject: [PATCH 17/46] comments on added methods --- provider/aci.go | 10 ++++++++++ provider/aci_test.go | 1 + 2 files changed, 11 insertions(+) diff --git a/provider/aci.go b/provider/aci.go index 88b4d839..8ee0eba0 100644 --- a/provider/aci.go +++ b/provider/aci.go @@ -1449,6 +1449,8 @@ func readDockerConfigJSONSecret(secret *v1.Secret, ips []aci.ImageRegistryCreden return ips, err } +//verify if Container is properly declared for the use on ACI +//this method is used for both initConainers and containers func (p *ACIProvider) verifyContainer(container *v1.Container) error { if len(container.Command) == 0 && len(container.Args) > 0 { return errdefs.InvalidInput("ACI does not support providing args without specifying the command. Please supply both command and args to the pod spec.") @@ -1456,10 +1458,14 @@ func (p *ACIProvider) verifyContainer(container *v1.Container) error { return nil } +//get the command declared on Container +//this method is used for both initConainers and containers func (p *ACIProvider) getCommand(container *v1.Container) []string { return append(container.Command, container.Args...) } +//get VolumeMounts declared on Container as []aci.VolumeMount +//this method is used for both initConainers and containers func (p *ACIProvider) getVolumeMounts(container *v1.Container) []aci.VolumeMount { volumeMounts := make([]aci.VolumeMount, 0, len(container.VolumeMounts)) for _, v := range container.VolumeMounts { @@ -1472,6 +1478,8 @@ func (p *ACIProvider) getVolumeMounts(container *v1.Container) []aci.VolumeMount return volumeMounts } +//get EnvironmentVariables declared on Container as []aci.EnvironmentVariable +//this method is used for both initConainers and containers func (p *ACIProvider) getEnvironmentVariables(container *v1.Container) []aci.EnvironmentVariable { environmentVariable := make([]aci.EnvironmentVariable, 0, len(container.Env)) for _, e := range container.Env { @@ -1483,6 +1491,7 @@ func (p *ACIProvider) getEnvironmentVariables(container *v1.Container) []aci.Env return environmentVariable } +//get InitContainers defined in Pod as []aci.InitContainerDefinition func (p *ACIProvider) getInitContainers(pod *v1.Pod) ([]aci.InitContainerDefinition, error) { initContainers := make([]aci.InitContainerDefinition, 0, len(pod.Spec.InitContainers)) for _, initContainer := range pod.Spec.InitContainers { @@ -1521,6 +1530,7 @@ func (p *ACIProvider) getInitContainers(pod *v1.Pod) ([]aci.InitContainerDefinit return initContainers, nil } +//get Containers defined in Pod as []aci.Container type func (p *ACIProvider) getContainers(pod *v1.Pod) ([]aci.Container, error) { containers := make([]aci.Container, 0, len(pod.Spec.Containers)) for _, container := range pod.Spec.Containers { diff --git a/provider/aci_test.go b/provider/aci_test.go index a6a46168..55303c00 100644 --- a/provider/aci_test.go +++ b/provider/aci_test.go @@ -1400,6 +1400,7 @@ func TestCreatePodWithCSIVolume(t *testing.T) { } } +//Test create InitContainers func TestCreatePodWithInitContainer(t *testing.T) { _, aciServerMocker, provider, err := prepareMocks() From 767a445b482778da7030ff1ef9ce01fed1f7962d Mon Sep 17 00:00:00 2001 From: t-ysalazar Date: Fri, 10 Jun 2022 11:56:36 -0700 Subject: [PATCH 18/46] complete unit test for ACI InitContainers --- provider/aci_test.go | 283 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 280 insertions(+), 3 deletions(-) diff --git a/provider/aci_test.go b/provider/aci_test.go index 55303c00..16710f08 100644 --- a/provider/aci_test.go +++ b/provider/aci_test.go @@ -1410,20 +1410,30 @@ func TestCreatePodWithInitContainer(t *testing.T) { podName := "pod-" + uuid.New().String() podNamespace := "ns-" + uuid.New().String() + azureFileVolumeName := "azure" aciServerMocker.OnCreate = func(subscription, resourceGroup, containerGroup string, cg *aci.ContainerGroup) (int, interface{}) { assert.Check(t, is.Equal(fakeSubscription, subscription), "Subscription doesn't match") assert.Check(t, is.Equal(fakeResourceGroup, resourceGroup), "Resource group doesn't match") + assert.Check(t, cg != nil, "Container group is nil") assert.Check(t, cg.ContainerGroupProperties.Containers != nil, "Containers should not be nil") assert.Check(t, cg.ContainerGroupProperties.InitContainers != nil, "InitContainer should not be nil") assert.Check(t, is.Equal(1, len(cg.ContainerGroupProperties.Containers)), "1 Container is expected") - assert.Check(t, is.Equal(2, len(cg.ContainerGroupProperties.InitContainers)), "2 InitContainer are expected") - assert.Check(t, is.Equal("nginx", cg.ContainerGroupProperties.Containers[0].Name), "Container nginx is expected") + assert.Check(t, is.Equal(2, len(cg.ContainerGroupProperties.InitContainers)), "2 InitContainers are expected") + + assert.Check(t, is.Equal("initContainer 01", cg.ContainerGroupProperties.InitContainers[0].Name), "first initContainer name must be \"initContainer 01\"") + assert.Check(t, is.Equal("alpine", cg.ContainerGroupProperties.InitContainers[0].Image), "InitContainer Image is not the expected") + assert.Check(t, cg.ContainerGroupProperties.InitContainers[0].VolumeMounts != nil, "VolumeMounts for InitContainer should not be nil") return http.StatusOK, cg } + fakeVolumeMount := v1.VolumeMount{ + Name: azureFileVolumeName, + MountPath: "/mnt/azure", + } + pod := &v1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: podName, @@ -1432,7 +1442,7 @@ func TestCreatePodWithInitContainer(t *testing.T) { Spec: v1.PodSpec{ Containers: []v1.Container{ { - Name: "nginx", + Name: "container name", }, }, InitContainers: []v1.Container{ @@ -1447,8 +1457,275 @@ func TestCreatePodWithInitContainer(t *testing.T) { }, }, } + pod.Spec.InitContainers[0].VolumeMounts = append(pod.Spec.InitContainers[0].VolumeMounts, fakeVolumeMount) if err := provider.CreatePod(context.Background(), pod); err != nil { t.Fatal("Failed to create pod", err) } } + +//Test Init Container with Ports, +//ACI does not support initContainer with ports, errors expected +func TestCreateInitContainerWithPort(t *testing.T) { + _, aciServerMocker, provider, err := prepareMocks() + + if err != nil { + t.Fatal("Unable to prepare the mocks", err) + } + + podName := "pod-" + uuid.New().String() + podNamespace := "ns-" + uuid.New().String() + + aciServerMocker.OnCreate = func(subscription, resourceGroup, containerGroup string, cg *aci.ContainerGroup) (int, interface{}) { + assert.Check(t, is.Equal(fakeSubscription, subscription), "Subscription doesn't match") + assert.Check(t, is.Equal(fakeResourceGroup, resourceGroup), "Resource group doesn't match") + return http.StatusOK, cg + } + + pod := &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: podName, + Namespace: podNamespace, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "container name", + }, + }, + InitContainers: []v1.Container{ + { + Name: "initContainer 01", + Ports: []v1.ContainerPort{ + { + Name: "port name", + ContainerPort: 80, + }, + }, + }, + }, + }, + } + + if err := provider.CreatePod(context.Background(), pod); err == nil { + t.Fatal("Ports for ACI initContainers are not supported", err) + } +} + +//Test Init Container with Resources specs, +//ACI does not support initContainer with Resources specs, errors expected +func TestCreateInitContainerWithResourcesSpecs(t *testing.T) { + _, aciServerMocker, provider, err := prepareMocks() + + if err != nil { + t.Fatal("Unable to prepare the mocks", err) + } + + podName := "pod-" + uuid.New().String() + podNamespace := "ns-" + uuid.New().String() + + aciServerMocker.OnCreate = func(subscription, resourceGroup, containerGroup string, cg *aci.ContainerGroup) (int, interface{}) { + assert.Check(t, is.Equal(fakeSubscription, subscription), "Subscription doesn't match") + assert.Check(t, is.Equal(fakeResourceGroup, resourceGroup), "Resource group doesn't match") + return http.StatusOK, cg + } + + pod := &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: podName, + Namespace: podNamespace, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "container name", + }, + }, + InitContainers: []v1.Container{ + { + Name: "initContainer 01", + Resources: v1.ResourceRequirements{ + Requests: v1.ResourceList{ + "cpu": resource.MustParse("1.981"), + "memory": resource.MustParse("3.49G"), + }, + Limits: v1.ResourceList{ + gpuResourceName: resource.MustParse("10"), + }, + }, + }, + }, + }, + } + + if err := provider.CreatePod(context.Background(), pod); err == nil { + t.Fatal("Resource specs for ACI initContainers are not supported", err) + } + + pod = &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: podName, + Namespace: podNamespace, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "container name", + }, + }, + InitContainers: []v1.Container{ + { + Name: "initContainer 01", + Resources: v1.ResourceRequirements{ + Limits: v1.ResourceList{ + gpuResourceName: resource.MustParse("10"), + }, + }, + }, + }, + }, + } + + if err := provider.CreatePod(context.Background(), pod); err == nil { + t.Fatal("Resource specs for ACI initContainers are not supported", err) + } + + pod = &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: podName, + Namespace: podNamespace, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "container name", + }, + }, + InitContainers: []v1.Container{ + { + Name: "initContainer 01", + Resources: v1.ResourceRequirements{ + Requests: v1.ResourceList{ + "cpu": resource.MustParse("1.981"), + "memory": resource.MustParse("3.49G"), + }, + }, + }, + }, + }, + } + + if err := provider.CreatePod(context.Background(), pod); err == nil { + t.Fatal("Resource specs for ACI initContainers are not supported", err) + } +} + +//Test Init Container with LivenessProbe, +//ACI does not support initContainer with LivenessProbe, errors expected +func TestCreateInitContainerWithLivenessProbe(t *testing.T) { + _, aciServerMocker, provider, err := prepareMocks() + + if err != nil { + t.Fatal("Unable to prepare the mocks", err) + } + + podName := "pod-" + uuid.New().String() + podNamespace := "ns-" + uuid.New().String() + + aciServerMocker.OnCreate = func(subscription, resourceGroup, containerGroup string, cg *aci.ContainerGroup) (int, interface{}) { + assert.Check(t, is.Equal(fakeSubscription, subscription), "Subscription doesn't match") + assert.Check(t, is.Equal(fakeResourceGroup, resourceGroup), "Resource group doesn't match") + return http.StatusOK, cg + } + + pod := &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: podName, + Namespace: podNamespace, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "container name", + }, + }, + InitContainers: []v1.Container{ + { + Name: "initContainer 01", + LivenessProbe: &v1.Probe{ + Handler: v1.Handler{ + HTTPGet: &v1.HTTPGetAction{ + Port: intstr.FromString("http"), + Path: "/", + }, + }, + InitialDelaySeconds: 10, + PeriodSeconds: 5, + TimeoutSeconds: 60, + SuccessThreshold: 3, + FailureThreshold: 5, + }, + }, + }, + }, + } + + if err := provider.CreatePod(context.Background(), pod); err == nil { + t.Fatal("LivenessProbe for ACI initContainers is not supported", err) + } +} + +//Test Init Container with ReadinessProbe, +//ACI does not support initContainer with ReadinessProbe, errors expected +func TestCreateInitContainerWithReadinessProbe(t *testing.T) { + _, aciServerMocker, provider, err := prepareMocks() + + if err != nil { + t.Fatal("Unable to prepare the mocks", err) + } + + podName := "pod-" + uuid.New().String() + podNamespace := "ns-" + uuid.New().String() + + aciServerMocker.OnCreate = func(subscription, resourceGroup, containerGroup string, cg *aci.ContainerGroup) (int, interface{}) { + assert.Check(t, is.Equal(fakeSubscription, subscription), "Subscription doesn't match") + assert.Check(t, is.Equal(fakeResourceGroup, resourceGroup), "Resource group doesn't match") + return http.StatusOK, cg + } + + pod := &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: podName, + Namespace: podNamespace, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "container name", + }, + }, + InitContainers: []v1.Container{ + { + Name: "initContainer 01", + ReadinessProbe: &v1.Probe{ + Handler: v1.Handler{ + HTTPGet: &v1.HTTPGetAction{ + Port: intstr.FromInt(8080), + Path: "/", + }, + }, + InitialDelaySeconds: 10, + PeriodSeconds: 5, + TimeoutSeconds: 60, + SuccessThreshold: 3, + FailureThreshold: 5, + }, + }, + }, + }, + } + + if err := provider.CreatePod(context.Background(), pod); err == nil { + t.Fatal("ReadinessProbe for ACI initContainers is not supported", err) + } +} From fd52af9f9e1747963fb7dc311711e42c2a817e39 Mon Sep 17 00:00:00 2001 From: t-ysalazar Date: Fri, 10 Jun 2022 13:59:03 -0700 Subject: [PATCH 19/46] Move the InitContainerDefinition closer to InitContainerProperties --- client/aci/types.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/client/aci/types.go b/client/aci/types.go index b42c6510..49ee0923 100644 --- a/client/aci/types.go +++ b/client/aci/types.go @@ -78,6 +78,15 @@ type InitContainerDefinition struct { InitContainerProperties `json:"properties,omitempty"` } +// InitContainerProperties is the initContainer instance properties. +type InitContainerProperties struct { + Image string `json:"image,omitempty"` + Command []string `json:"command,omitempty"` + EnvironmentVariables []EnvironmentVariable `json:"environmentVariables,omitempty"` + InstanceView ContainerPropertiesInstanceView `json:"instanceView,omitempty"` + VolumeMounts []VolumeMount `json:"volumeMounts,omitempty"` +} + // ContainerGroup is a container group. type ContainerGroup struct { api.ResponseMetadata `json:"-"` @@ -144,15 +153,6 @@ type ContainerProperties struct { ReadinessProbe *ContainerProbe `json:"readinessProbe,omitempty"` } -// InitContainerProperties is the initContainer instance properties. -type InitContainerProperties struct { - Image string `json:"image,omitempty"` - Command []string `json:"command,omitempty"` - EnvironmentVariables []EnvironmentVariable `json:"environmentVariables,omitempty"` - InstanceView ContainerPropertiesInstanceView `json:"instanceView,omitempty"` - VolumeMounts []VolumeMount `json:"volumeMounts,omitempty"` -} - // ContainerPropertiesInstanceView is the instance view of the container instance. Only valid in response. type ContainerPropertiesInstanceView struct { RestartCount int32 `json:"restartCount,omitempty"` From bdf2cc12341b1c5ed2eef478145c6782bf4d6eed Mon Sep 17 00:00:00 2001 From: t-ysalazar Date: Mon, 13 Jun 2022 14:15:36 -0700 Subject: [PATCH 20/46] Revert "ignore helm init and pod specs example" This reverts commit e979e998eba0e09eaf31bfc209164ed7afc15e9b. --- .gitignore | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.gitignore b/.gitignore index 94fc75f5..b66fb05f 100644 --- a/.gitignore +++ b/.gitignore @@ -4,10 +4,6 @@ *.dll *.so *.dylib -*.bash - -helm-start.sh -podspcs.yaml # Test binary, build with `go test -c` *.test From a161e72b1eae53a41e93a4ca5731ed0bca1c4336 Mon Sep 17 00:00:00 2001 From: t-ysalazar Date: Tue, 5 Jul 2022 14:41:40 -0700 Subject: [PATCH 21/46] e2e for init containers --- e2e/fixtures/initcontainers_pod.yml | 37 +++++++++++++++++++++++ e2e/pods_test.go | 46 ++++++++++++++++++----------- 2 files changed, 65 insertions(+), 18 deletions(-) create mode 100644 e2e/fixtures/initcontainers_pod.yml diff --git a/e2e/fixtures/initcontainers_pod.yml b/e2e/fixtures/initcontainers_pod.yml new file mode 100644 index 00000000..1e9834d4 --- /dev/null +++ b/e2e/fixtures/initcontainers_pod.yml @@ -0,0 +1,37 @@ +apiVersion: v1 +kind: Namespace +metadata: + name: vk-test +--- +apiVersion: v1 +kind: Pod +metadata: + name: vk-e2e-initcontainers + namespace: vk-test +spec: + initContainers: + - image: alpine + name: init-container-01 + command: [ "/bin/sh" ] + args: [ "-c", "echo \"Hi from initcontainer 01\"; done" ] + - image: alpine + name: init-container-02 + command: [ "/bin/sh" ] + args: [ "-c", "echo \"Hi from initcontainer 02\"; done" ] + containers: + - image: alpine + imagePullPolicy: Always + name: container + command: [ "/bin/sh" ] + args: [ "-c", "sTimeout=10; xMax=10; x=1; while [ $x -le $((xMax*(60/sTimeout))) ]; do echo \"HOLA $x time for $((sTimeout))s \" $(( x++ )) \" => $(date +%Y-%m-%d_%H:%M:%S)\"; sleep $sTimeout; done" ] + resources: + requests: + memory: 1G + cpu: 1 + nodeSelector: + kubernetes.io/role: agent + beta.kubernetes.io/os: linux + type: virtual-kubelet + tolerations: + - key: virtual-kubelet.io/provider + operator: Exists diff --git a/e2e/pods_test.go b/e2e/pods_test.go index fc767374..5dcc3b90 100644 --- a/e2e/pods_test.go +++ b/e2e/pods_test.go @@ -1,22 +1,12 @@ package e2e import ( - "bytes" "testing" "time" ) -func TestPodLifecycle(t *testing.T) { - // delete the pod first - kubectl("delete", "pod/vk-e2e-hpa") - - spec, err := fixtures.ReadFile("fixtures/hpa.yml") - if err != nil { - t.Fatal(err) - } - - cmd := kubectl("apply", "-f", "-") - cmd.Stdin = bytes.NewReader(spec) +func CreatePodFromKubectl(t *testing.T, podName string, podDir string) { + cmd := kubectl("apply", "-f", podDir) if out, err := cmd.CombinedOutput(); err != nil { t.Fatal(string(out)) } @@ -26,20 +16,22 @@ func TestPodLifecycle(t *testing.T) { if !ok { timeout = 300 * time.Second } - cmd = kubectl("wait", "--for=condition=ready", "--timeout="+timeout.String(), "pod/vk-e2e-hpa", "--namespace=vk-test") + cmd = kubectl("wait", "--for=condition=ready", "--timeout="+timeout.String(), "pod/"+podName, "--namespace=vk-test") if out, err := cmd.CombinedOutput(); err != nil { t.Fatal(string(out)) } + t.Log("success create pod") +} - // query metrics - deadline = time.Now().Add(5 * time.Minute) +func QueryKubectlMetrics(t *testing.T, podName string) { + deadline := time.Now().Add(5 * time.Minute) for { t.Log("query metrics ....") - cmd = kubectl("get", "--raw", "/apis/metrics.k8s.io/v1beta1/namespaces/vk-test/pods/vk-e2e-hpa") + cmd := kubectl("get", "--raw", "/apis/metrics.k8s.io/v1beta1/namespaces/vk-test/pods/"+podName) out, err := cmd.CombinedOutput() if time.Now().After(deadline) { - t.Fatal("failed to query pod's stats from metris server API") + t.Fatal("failed to query pod's stats from metrics server API") } if err == nil { t.Logf("success query metrics %s", string(out)) @@ -47,10 +39,28 @@ func TestPodLifecycle(t *testing.T) { } time.Sleep(10 * time.Second) } +} +func CleanPodFromKubectl(t *testing.T, podName string) { t.Log("clean up pod") - cmd = kubectl("delete", "pod/vk-e2e-hpa", "--namespace=vk-test") + cmd := kubectl("delete", "pod/"+podName, "--namespace=vk-test") if out, err := cmd.CombinedOutput(); err != nil { t.Fatal(string(out)) } } + +func TestPodLifecycle(t *testing.T) { + kubectl("delete", "pod/vk-e2e-hpa", "--namespace=vk-test") + + CreatePodFromKubectl(t, "vk-e2e-hpa", "fixtures/hpa.yml") + //QueryKubectlMetrics(t, "vk-e2e-hpa") + CleanPodFromKubectl(t, "vk-e2e-hpa") +} + +func TestInitContainerPod(t *testing.T) { + kubectl("delete", "pod/vk-e2e-initcontainers", "--namespace=vk-test") + + CreatePodFromKubectl(t, "vk-e2e-initcontainers", "fixtures/initcontainers_pod.yml") + //QueryKubectlMetrics(t, "vk-e2e-initcontainers") + CleanPodFromKubectl(t, "vk-e2e-initcontainers") +} From bd4ec999a015703967c9a39aab64cc81cfdaf779 Mon Sep 17 00:00:00 2001 From: t-ysalazar Date: Tue, 5 Jul 2022 14:43:40 -0700 Subject: [PATCH 22/46] e2e for init containers --- e2e/pods_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/e2e/pods_test.go b/e2e/pods_test.go index 5dcc3b90..6c81e42e 100644 --- a/e2e/pods_test.go +++ b/e2e/pods_test.go @@ -53,7 +53,7 @@ func TestPodLifecycle(t *testing.T) { kubectl("delete", "pod/vk-e2e-hpa", "--namespace=vk-test") CreatePodFromKubectl(t, "vk-e2e-hpa", "fixtures/hpa.yml") - //QueryKubectlMetrics(t, "vk-e2e-hpa") + QueryKubectlMetrics(t, "vk-e2e-hpa") CleanPodFromKubectl(t, "vk-e2e-hpa") } @@ -61,6 +61,6 @@ func TestInitContainerPod(t *testing.T) { kubectl("delete", "pod/vk-e2e-initcontainers", "--namespace=vk-test") CreatePodFromKubectl(t, "vk-e2e-initcontainers", "fixtures/initcontainers_pod.yml") - //QueryKubectlMetrics(t, "vk-e2e-initcontainers") + QueryKubectlMetrics(t, "vk-e2e-initcontainers") CleanPodFromKubectl(t, "vk-e2e-initcontainers") } From 443468c831d83c04a004f5ed6f49322e8b5be691 Mon Sep 17 00:00:00 2001 From: t-ysalazar Date: Tue, 5 Jul 2022 14:55:23 -0700 Subject: [PATCH 23/46] e2e for init containers, move podName and podDir --- e2e/pods_test.go | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/e2e/pods_test.go b/e2e/pods_test.go index 6c81e42e..6b369b61 100644 --- a/e2e/pods_test.go +++ b/e2e/pods_test.go @@ -50,17 +50,23 @@ func CleanPodFromKubectl(t *testing.T, podName string) { } func TestPodLifecycle(t *testing.T) { - kubectl("delete", "pod/vk-e2e-hpa", "--namespace=vk-test") + podName := "vk-e2e-hpa" + podDir := "fixtures/hpa.yml" - CreatePodFromKubectl(t, "vk-e2e-hpa", "fixtures/hpa.yml") - QueryKubectlMetrics(t, "vk-e2e-hpa") - CleanPodFromKubectl(t, "vk-e2e-hpa") + kubectl("delete", "pod/"+podName, "--namespace=vk-test") + + CreatePodFromKubectl(t, podName, podDir) + QueryKubectlMetrics(t, podName) + CleanPodFromKubectl(t, podName) } func TestInitContainerPod(t *testing.T) { - kubectl("delete", "pod/vk-e2e-initcontainers", "--namespace=vk-test") + podName := "vk-e2e-initcontainers" + podDir := "fixtures/initcontainers_pod.yml" + + kubectl("delete", "pod/"+podName, "--namespace=vk-test") - CreatePodFromKubectl(t, "vk-e2e-initcontainers", "fixtures/initcontainers_pod.yml") - QueryKubectlMetrics(t, "vk-e2e-initcontainers") - CleanPodFromKubectl(t, "vk-e2e-initcontainers") + CreatePodFromKubectl(t, podName, podDir) + QueryKubectlMetrics(t, podName) + CleanPodFromKubectl(t, podName) } From 961171e6f4f330c556ebe712b31f63103dd1d8c6 Mon Sep 17 00:00:00 2001 From: t-ysalazar Date: Tue, 5 Jul 2022 15:20:24 -0700 Subject: [PATCH 24/46] e2e comments --- e2e/pods_test.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/e2e/pods_test.go b/e2e/pods_test.go index 6b369b61..54845886 100644 --- a/e2e/pods_test.go +++ b/e2e/pods_test.go @@ -5,6 +5,7 @@ import ( "time" ) +//create the pod 'podName' with the pod specs on 'podDir' func CreatePodFromKubectl(t *testing.T, podName string, podDir string) { cmd := kubectl("apply", "-f", podDir) if out, err := cmd.CombinedOutput(); err != nil { @@ -24,6 +25,7 @@ func CreatePodFromKubectl(t *testing.T, podName string, podDir string) { t.Log("success create pod") } +//query the metrics of the pod func QueryKubectlMetrics(t *testing.T, podName string) { deadline := time.Now().Add(5 * time.Minute) for { @@ -41,7 +43,8 @@ func QueryKubectlMetrics(t *testing.T, podName string) { } } -func CleanPodFromKubectl(t *testing.T, podName string) { +//delete pod +func DeletePodFromKubectl(t *testing.T, podName string) { t.Log("clean up pod") cmd := kubectl("delete", "pod/"+podName, "--namespace=vk-test") if out, err := cmd.CombinedOutput(); err != nil { @@ -57,9 +60,10 @@ func TestPodLifecycle(t *testing.T) { CreatePodFromKubectl(t, podName, podDir) QueryKubectlMetrics(t, podName) - CleanPodFromKubectl(t, podName) + DeletePodFromKubectl(t, podName) } +//e2e test for create a pod with init containers func TestInitContainerPod(t *testing.T) { podName := "vk-e2e-initcontainers" podDir := "fixtures/initcontainers_pod.yml" @@ -68,5 +72,5 @@ func TestInitContainerPod(t *testing.T) { CreatePodFromKubectl(t, podName, podDir) QueryKubectlMetrics(t, podName) - CleanPodFromKubectl(t, podName) + DeletePodFromKubectl(t, podName) } From 5daedd104efe0bbfda6190698f839a7f61cf8ad3 Mon Sep 17 00:00:00 2001 From: t-ysalazar Date: Wed, 6 Jul 2022 15:04:38 -0700 Subject: [PATCH 25/46] fix pod specs for init containers in e2e --- e2e/pods_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e/pods_test.go b/e2e/pods_test.go index 54845886..54312d9a 100644 --- a/e2e/pods_test.go +++ b/e2e/pods_test.go @@ -63,7 +63,7 @@ func TestPodLifecycle(t *testing.T) { DeletePodFromKubectl(t, podName) } -//e2e test for create a pod with init containers +//e2e lifecycle test for a pod with init containers func TestInitContainerPod(t *testing.T) { podName := "vk-e2e-initcontainers" podDir := "fixtures/initcontainers_pod.yml" From 5ef36bcf2a0e9316055a0eb1d7586b50ac9f31de Mon Sep 17 00:00:00 2001 From: t-ysalazar Date: Wed, 6 Jul 2022 15:08:09 -0700 Subject: [PATCH 26/46] fix pod specs for init containers in e2e --- e2e/fixtures/initcontainers_pod.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/e2e/fixtures/initcontainers_pod.yml b/e2e/fixtures/initcontainers_pod.yml index 1e9834d4..19a2596c 100644 --- a/e2e/fixtures/initcontainers_pod.yml +++ b/e2e/fixtures/initcontainers_pod.yml @@ -13,17 +13,17 @@ spec: - image: alpine name: init-container-01 command: [ "/bin/sh" ] - args: [ "-c", "echo \"Hi from initcontainer 01\"; done" ] + args: [ "-c", "echo \"Hi\"" ] - image: alpine name: init-container-02 command: [ "/bin/sh" ] - args: [ "-c", "echo \"Hi from initcontainer 02\"; done" ] + args: [ "-c", "echo \"Hi\"" ] containers: - image: alpine imagePullPolicy: Always name: container command: [ "/bin/sh" ] - args: [ "-c", "sTimeout=10; xMax=10; x=1; while [ $x -le $((xMax*(60/sTimeout))) ]; do echo \"HOLA $x time for $((sTimeout))s \" $(( x++ )) \" => $(date +%Y-%m-%d_%H:%M:%S)\"; sleep $sTimeout; done" ] + args: [ "-c", "sTimeout=10; xMax=10; x=1; while [ $x -le $((xMax*(60/sTimeout))) ]; do echo \"Hi $x time for $((sTimeout))s \" $(( x++ )) \" => $(date +%Y-%m-%d_%H:%M:%S)\"; sleep $sTimeout; done" ] resources: requests: memory: 1G From f97e251166e3bbe9bb5fe1f16bcdf8051aeb76ea Mon Sep 17 00:00:00 2001 From: Arnav Arnav Date: Wed, 17 Aug 2022 18:24:58 -0400 Subject: [PATCH 27/46] added init containers usage to README --- README.md | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index ec11cbbf..22c29c3d 100644 --- a/README.md +++ b/README.md @@ -22,7 +22,7 @@ This document details configuring the Virtual Kubelet ACI provider. Virtual Kubelet's ACI provider relies heavily on the feature set that Azure Container Instances provide. Please check the Azure documentation accurate details on region availability, pricing and new features. The list here attempts to give an accurate reference for the features we support in ACI and the ACI provider within Virtual Kubelet. -### features +### Features * Volumes: empty dir, github repo, Azure Files * Secure env variables, config maps @@ -31,6 +31,7 @@ Virtual Kubelet's ACI provider relies heavily on the feature set that Azure Cont * Basic Azure Networking support within AKS virtual node * [Exec support](https://docs.microsoft.com/azure/container-instances/container-instances-exec) for container instances * Azure Monitor integration or formally known as OMS +* Support for init-containers ([use init containers](#Create-pod-with-init-containers)) ### Limitations @@ -644,6 +645,28 @@ Output: ``` --> +### Create pod with init containers +Multiple init containers can be specified in the podspec similar to how containers are specified + +```yaml +spec: + initContainers: + - image: + name: init-container-01 + command: [ "/bin/sh" ] + args: [ "-c", "echo \"Hi\"" ] + - image: + name: init-container-02 + command: [ "/bin/sh" ] + args: [ "-c", "echo \"Hi\"" ] + containers: + - image: + imagePullPolicy: Always + name: container + command: [ "/bin/sh" ] +``` +More information on init containers can be found in [Kubernetes](https://kubernetes.io/docs/concepts/workloads/pods/init-containers/) and [ACI](https://docs.microsoft.com/en-us/azure/container-instances/container-instances-init-container) documentations + ## Work around for the virtual kubelet pod If your pod that's scheduled onto the Virtual Kubelet node is in a pending state please add this workaround to your Virtual Kubelet pod spec. From 1040b31ccf4d8287f3d7d8caaaa63ce67698c03c Mon Sep 17 00:00:00 2001 From: Arnav Arnav Date: Tue, 13 Sep 2022 18:49:51 -0400 Subject: [PATCH 28/46] use separate test for init container for clean namespace --- e2e/fixtures/initcontainers_pod.yml | 7 +- e2e/pods_test.go | 132 +++++++++++++++++----------- hack/e2e/aks.sh | 4 +- 3 files changed, 89 insertions(+), 54 deletions(-) diff --git a/e2e/fixtures/initcontainers_pod.yml b/e2e/fixtures/initcontainers_pod.yml index c934dcb1..2878aed1 100644 --- a/e2e/fixtures/initcontainers_pod.yml +++ b/e2e/fixtures/initcontainers_pod.yml @@ -17,8 +17,11 @@ spec: - image: alpine imagePullPolicy: Always name: container - command: [ "/bin/sh" ] - args: [ "-c", "sTimeout=10; xMax=10; x=1; while [ $x -le $((xMax*(60/sTimeout))) ]; do echo \"Hi $x time for $((sTimeout))s \" $(( x++ )) \" => $(date +%Y-%m-%d_%H:%M:%S)\"; sleep $sTimeout; done" ] + command: [ + "sh", + "-c", + "while sleep 10; do echo pod with init container; done;" + ] resources: requests: memory: 1G diff --git a/e2e/pods_test.go b/e2e/pods_test.go index d2fe54f2..31cc5348 100644 --- a/e2e/pods_test.go +++ b/e2e/pods_test.go @@ -29,59 +29,89 @@ func TestPodLifecycle(t *testing.T) { t.Fatal(string(out)) } - testCases := []E2EPodTestCase{ - { - name: "basic pod", - pod: E2EPodInfo{ - name: "vk-e2e-hpa", - dir: "fixtures/hpa.yml", - }, - }, - { - name: "pod with init container", - pod: E2EPodInfo{ - name: "vk-e2e-initcontainers", - dir: "fixtures/initcontainers_pod.yml", - }, - }, - } - - for _, test := range testCases { - t.Run(test.name, func(t *testing.T) { - cmd = kubectl("apply", "-f", test.pod.dir, "--namespace=vk-test") - if out, err := cmd.CombinedOutput(); err != nil { - t.Fatal(string(out)) - } - - deadline, ok := t.Deadline() - timeout := time.Until(deadline) - if !ok { - timeout = 300 * time.Second - } - cmd = kubectl("wait", "--for=condition=ready", "--timeout="+timeout.String(), "pod/"+test.pod.name, "--namespace=vk-test") - if out, err := cmd.CombinedOutput(); err != nil { - t.Fatal(string(out)) - } - t.Log("success create pod") - - // query metrics - deadline = time.Now().Add(5 * time.Minute) - for { - t.Log("query metrics ....") - cmd = kubectl("get", "--raw", "/apis/metrics.k8s.io/v1beta1/namespaces/vk-test/pods/"+test.pod.name) - out, err := cmd.CombinedOutput() - if time.Now().After(deadline) { - t.Fatal("failed to query pod's stats from metrics server API") - } - if err == nil { - t.Logf("success query metrics %s", string(out)) - break - } - time.Sleep(10 * time.Second) - } - }) + cmd = kubectl("apply", "-f", "fixtures/hpa.yml", "--namespace=vk-test") + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatal(string(out)) + } + + deadline, ok := t.Deadline() + timeout := time.Until(deadline) + if !ok { + timeout = 300 * time.Second + } + cmd = kubectl("wait", "--for=condition=ready", "--timeout="+timeout.String(), "pod/vk-e2e-hpa", "--namespace=vk-test") + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatal(string(out)) } + t.Log("success create pod") + // query metrics + deadline = time.Now().Add(5 * time.Minute) + for { + t.Log("query metrics ....") + cmd = kubectl("get", "--raw", "/apis/metrics.k8s.io/v1beta1/namespaces/vk-test/pods/vk-e2e-hpa") + out, err := cmd.CombinedOutput() + if time.Now().After(deadline) { + t.Fatal("failed to query pod's stats from metrics server API") + } + if err == nil { + t.Logf("success query metrics %s", string(out)) + break + } + time.Sleep(10 * time.Second) + } + + t.Log("clean up") + cmd = kubectl("delete", "namespace", "vk-test", "--ignore-not-found") + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatal(string(out)) + } +} + +func TestPodWithInitContainers(t *testing.T) { + // delete the namespace first + cmd := kubectl("delete", "namespace", "vk-test", "--ignore-not-found") + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatal(string(out)) + } + + // create namespace + cmd = kubectl("apply", "-f", "fixtures/namespace.yml") + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatal(string(out)) + } + + cmd = kubectl("apply", "-f", "fixtures/initcontainers_pod.yml", "--namespace=vk-test") + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatal(string(out)) + } + + deadline, ok := t.Deadline() + timeout := time.Until(deadline) + if !ok { + timeout = 300 * time.Second + } + cmd = kubectl("wait", "--for=condition=ready", "--timeout="+timeout.String(), "pod/vk-e2e-initcontainers", "--namespace=vk-test") + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatal(string(out)) + } + t.Log("success create pod") + + // query metrics + deadline = time.Now().Add(10 * time.Minute) + for { + t.Log("query metrics ....") + cmd = kubectl("get", "--raw", "/apis/metrics.k8s.io/v1beta1/namespaces/vk-test/pods/vk-e2e-initcontainers") + out, err := cmd.CombinedOutput() + if time.Now().After(deadline) { + t.Fatal("failed to query pod's stats from metrics server API") + } + if err == nil { + t.Logf("success query metrics %s", string(out)) + break + } + time.Sleep(10 * time.Second) + } t.Log("clean up") cmd = kubectl("delete", "namespace", "vk-test", "--ignore-not-found") if out, err := cmd.CombinedOutput(); err != nil { diff --git a/hack/e2e/aks.sh b/hack/e2e/aks.sh index 740e0982..8b7cd2fb 100755 --- a/hack/e2e/aks.sh +++ b/hack/e2e/aks.sh @@ -20,7 +20,7 @@ if [ "$PR_RAND" = "" ]; then fi : "${RESOURCE_GROUP:=vk-aci-test-$RANDOM_NUM}" -: "${LOCATION:=westus2}" +: "${LOCATION:=eastus2}" : "${CLUSTER_NAME:=${RESOURCE_GROUP}}" : "${NODE_COUNT:=1}" : "${CHART_NAME:=vk-aci-test-aks}" @@ -182,6 +182,8 @@ helm install \ --set "providers.azure.vnet.clusterCidr=$CLUSTER_SUBNET_RANGE" \ --set "providers.azure.vnet.kubeDnsIp=$KUBE_DNS_IP" \ --set "providers.azure.masterUri=$MASTER_URI" \ + --set "providers.azure.aciResourceGroup=${RESOURCE_GROUP}" \ + --set "providers.azure.aciRegion=${LOCATION}" \ "$CHART_NAME" \ ./charts/virtual-kubelet From bfe957bc1784f7fd3ee5985d3d4f401aaec8e2cb Mon Sep 17 00:00:00 2001 From: Arnav Arnav Date: Tue, 13 Sep 2022 18:56:33 -0400 Subject: [PATCH 29/46] remove unused code --- e2e/pods_test.go | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/e2e/pods_test.go b/e2e/pods_test.go index 31cc5348..8c664b18 100644 --- a/e2e/pods_test.go +++ b/e2e/pods_test.go @@ -6,16 +6,6 @@ import ( "time" ) -type E2EPodTestCase struct { - name string - pod E2EPodInfo -} - -type E2EPodInfo struct { - name string - dir string -} - func TestPodLifecycle(t *testing.T) { // delete the namespace first cmd := kubectl("delete", "namespace", "vk-test", "--ignore-not-found") From 6f547ea043f919d10b6cd79c9704148c826ae304 Mon Sep 17 00:00:00 2001 From: Arnav Arnav Date: Wed, 14 Sep 2022 19:48:19 -0400 Subject: [PATCH 30/46] removed rg from helm install since role isn't present --- hack/e2e/aks.sh | 2 -- 1 file changed, 2 deletions(-) diff --git a/hack/e2e/aks.sh b/hack/e2e/aks.sh index 8b7cd2fb..5dfa97c2 100755 --- a/hack/e2e/aks.sh +++ b/hack/e2e/aks.sh @@ -182,8 +182,6 @@ helm install \ --set "providers.azure.vnet.clusterCidr=$CLUSTER_SUBNET_RANGE" \ --set "providers.azure.vnet.kubeDnsIp=$KUBE_DNS_IP" \ --set "providers.azure.masterUri=$MASTER_URI" \ - --set "providers.azure.aciResourceGroup=${RESOURCE_GROUP}" \ - --set "providers.azure.aciRegion=${LOCATION}" \ "$CHART_NAME" \ ./charts/virtual-kubelet From 3d72f730c050f7dd6326f8170a28f6bdefd32286 Mon Sep 17 00:00:00 2001 From: Arnav Arnav Date: Fri, 28 Oct 2022 16:59:03 -0700 Subject: [PATCH 31/46] added initContainer using sdk --- pkg/provider/aci.go | 87 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/pkg/provider/aci.go b/pkg/provider/aci.go index a5267878..3847a038 100644 --- a/pkg/provider/aci.go +++ b/pkg/provider/aci.go @@ -484,7 +484,15 @@ func (p *ACIProvider) CreatePod(ctx context.Context, pod *v1.Pod) error { return err } + + // get initContainers + initContainers, err := p.getInitContainers(pod) + if err != nil { + return err + } + // assign all the things + cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.InitContainers = &initContainers cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Containers = containers cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Volumes = &volumes cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.ImageRegistryCredentials = creds @@ -1213,6 +1221,85 @@ func readDockerConfigJSONSecret(secret *v1.Secret, ips []azaci.ImageRegistryCred return ips, err } +//verify if Container is properly declared for the use on ACI +func (p *ACIProvider) verifyContainer(container *v1.Container) error { + if len(container.Command) == 0 && len(container.Args) > 0 { + return errdefs.InvalidInput("ACI does not support providing args without specifying the command. Please supply both command and args to the pod spec.") + } + return nil +} + +//this method is used for both initConainers and containers +func (p *ACIProvider) getCommand(container *v1.Container) *[]string { + command := append(container.Command, container.Args...) + return &command +} + +//get VolumeMounts declared on Container as []aci.VolumeMount +func (p *ACIProvider) getVolumeMounts(container *v1.Container) *[]azaci.VolumeMount { + volumeMounts := make([]azaci.VolumeMount, 0, len(container.VolumeMounts)) + for _, v := range container.VolumeMounts { + volumeMounts = append(volumeMounts, azaci.VolumeMount{ + Name: &v.Name, + MountPath: &v.MountPath, + ReadOnly: &v.ReadOnly, + }) + } + return &volumeMounts +} + +//get EnvironmentVariables declared on Container as []aci.EnvironmentVariable +func (p *ACIProvider) getEnvironmentVariables(container *v1.Container) *[]azaci.EnvironmentVariable { + environmentVariable := make([]azaci.EnvironmentVariable, 0, len(container.Env)) + for _, e := range container.Env { + if e.Value != "" { + envVar := getACIEnvVar(e) + environmentVariable = append(environmentVariable, envVar) + } + } + return &environmentVariable +} + +//get InitContainers defined in Pod as []aci.InitContainerDefinition +func (p *ACIProvider) getInitContainers(pod *v1.Pod) ([]azaci.InitContainerDefinition, error) { + initContainers := make([]azaci.InitContainerDefinition, 0, len(pod.Spec.InitContainers)) + for _, initContainer := range pod.Spec.InitContainers { + err := p.verifyContainer(&initContainer) + if err != nil { + return nil, err + } + + if initContainer.Ports != nil { + return nil, errdefs.InvalidInput("ACI initContainers does not support ports.") + } + if initContainer.Resources.Requests != nil { + return nil, errdefs.InvalidInput("ACI initContainers does not support resources requests.") + } + if initContainer.Resources.Limits != nil { + return nil, errdefs.InvalidInput("ACI initContainers does not support resources limits.") + } + if initContainer.LivenessProbe != nil { + return nil, errdefs.InvalidInput("ACI initContainers does not support livenessProbe.") + } + if initContainer.ReadinessProbe != nil { + return nil, errdefs.InvalidInput("ACI initContainers does not support readinessProbe.") + } + + newInitContainer := azaci.InitContainerDefinition{ + Name: &initContainer.Name, + InitContainerPropertiesDefinition: &azaci.InitContainerPropertiesDefinition { + Image: &initContainer.Image, + Command: p.getCommand(&initContainer), + VolumeMounts: p.getVolumeMounts(&initContainer), + EnvironmentVariables: p.getEnvironmentVariables(&initContainer), + }, + } + + initContainers = append(initContainers, newInitContainer) + } + return initContainers, nil +} + func (p *ACIProvider) getContainers(pod *v1.Pod) (*[]azaci.Container, error) { containers := make([]azaci.Container, 0, len(pod.Spec.Containers)) From 9e7fd19c08a4b1384ed827a1069f34c52f8648ce Mon Sep 17 00:00:00 2001 From: Arnav Arnav Date: Mon, 31 Oct 2022 10:24:14 -0700 Subject: [PATCH 32/46] added unit tests for init containers --- pkg/provider/aci_init_container_test.go | 198 ++++++++++++++++++++++++ 1 file changed, 198 insertions(+) create mode 100644 pkg/provider/aci_init_container_test.go diff --git a/pkg/provider/aci_init_container_test.go b/pkg/provider/aci_init_container_test.go new file mode 100644 index 00000000..ab306029 --- /dev/null +++ b/pkg/provider/aci_init_container_test.go @@ -0,0 +1,198 @@ +package provider + + +import ( + "context" + "fmt" + "testing" + + "github.com/virtual-kubelet/virtual-kubelet/errdefs" + "github.com/golang/mock/gomock" + "github.com/virtual-kubelet/azure-aci/pkg/client" + "github.com/virtual-kubelet/node-cli/manager" + "gotest.tools/assert" + is "gotest.tools/assert/cmp" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/apimachinery/pkg/api/resource" +) + +func TestCreatePodWithInitContainers(t *testing.T) { + + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + aciMocks := createNewACIMock() + aciMocks.MockCreateContainerGroup = func(ctx context.Context, resourceGroup, podNS, podName string, cg *client.ContainerGroupWrapper) error { + containers := *cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Containers + initContainers := *cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.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") + assert.Check(t, is.Equal(len(containers), 1), "1 Container is expected") + assert.Check(t, is.Equal(len(initContainers), 2), "2 init containers are expected") + assert.Check(t, initContainers[0].VolumeMounts != nil, "Volume mount should be present") + assert.Check(t, initContainers[0].EnvironmentVariables != nil, "Volume mount should be present") + assert.Check(t, initContainers[0].Command != nil, "Command mount should be present") + assert.Check(t, initContainers[0].Image != nil, "Image should be present") + + return nil + } + + + pod := &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: podName, + Namespace: podNamespace, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "container name", + }, + }, + }, + } + cases := []struct { + description string + initContainers []v1.Container + expectedError error + }{ + { + description: "Init Containers with Supported fields", + expectedError: nil, + initContainers: []v1.Container{ + v1.Container{ + Name: "initContainer 01", + Image: "alpine", + VolumeMounts: []v1.VolumeMount{ + v1.VolumeMount{ + Name: "fakeVolumeName", + MountPath: "/mnt/azure", + }, + }, + Command: []string{"/bin/bash"}, + Args: []string{"-c echo test"}, + Env: []v1.EnvVar{ + v1.EnvVar{ + Name: "TEST_ENV", + Value: "testvalue", + }, + }, + }, + v1.Container{ + Name: "initContainer 02", + Image: "alpine", + }, + }, + }, + { + description: "Init Containers with ports", + initContainers: []v1.Container{ + v1.Container{ + Name: "initContainer 01", + Image: "alpine", + Ports: []v1.ContainerPort{ + v1.ContainerPort{ + Name: "http", + ContainerPort: 80, + Protocol: "TCP", + }, + }, + }, + }, + expectedError: errdefs.InvalidInput("ACI initContainers does not support ports."), + }, + { + description: "Init Containers with liveness probe", + initContainers: []v1.Container{ + v1.Container{ + Name: "initContainer 01", + LivenessProbe: &v1.Probe{ + Handler: v1.Handler{ + HTTPGet: &v1.HTTPGetAction{ + Port: intstr.FromString("http"), + Path: "/", + }, + }, + InitialDelaySeconds: 10, + PeriodSeconds: 5, + TimeoutSeconds: 60, + SuccessThreshold: 3, + FailureThreshold: 5, + }, + }, + }, + expectedError: errdefs.InvalidInput("ACI initContainers does not support livenessProbe."), + }, + { + description: "Init Containers with readiness probe", + initContainers: []v1.Container{ + v1.Container{ + Name: "initContainer 01", + ReadinessProbe: &v1.Probe{ + Handler: v1.Handler{ + HTTPGet: &v1.HTTPGetAction{ + Port: intstr.FromInt(8080), + Path: "/", + }, + }, + InitialDelaySeconds: 10, + PeriodSeconds: 5, + TimeoutSeconds: 60, + SuccessThreshold: 3, + FailureThreshold: 5, + }, + }, + }, + expectedError: errdefs.InvalidInput("ACI initContainers does not support readinessProbe."), + }, + { + description: "Init Containers with resource request", + initContainers: []v1.Container{ + { + Name: "initContainer 01", + Resources: v1.ResourceRequirements{ + Requests: v1.ResourceList{ + "cpu": resource.MustParse("1.981"), + "memory": resource.MustParse("3.49G"), + }, + Limits: v1.ResourceList{ + gpuResourceName: resource.MustParse("10"), + }, + }, + }, + }, + expectedError: errdefs.InvalidInput("ACI initContainers does not support resources requests."), + }, + } + for _, tc := range cases { + t.Run(tc.description, func(t *testing.T) { + + resourceManager, err := manager.NewResourceManager( + NewMockPodLister(mockCtrl), + NewMockSecretLister(mockCtrl), + NewMockConfigMapLister(mockCtrl), + NewMockServiceLister(mockCtrl), + NewMockPersistentVolumeClaimLister(mockCtrl), + NewMockPersistentVolumeLister(mockCtrl)) + if err != nil { + t.Fatal("Unable to prepare the mocks for resourceManager", err) + } + + provider, err := createTestProvider(aciMocks, resourceManager) + if err != nil { + t.Fatal("Unable to create test provider", err) + } + + pod.Spec.InitContainers = tc.initContainers + err = provider.CreatePod(context.Background(), pod) + + // check that the correct error is returned + if tc.expectedError != nil && err != tc.expectedError { + assert.Equal(t, tc.expectedError.Error(), err.Error(), "expected error and actual error don't match") + } + }) + } +} From 0164951398bc537d4a7b830371185a8fc87cb574 Mon Sep 17 00:00:00 2001 From: Arnav Arnav Date: Mon, 31 Oct 2022 10:55:24 -0700 Subject: [PATCH 33/46] renoved duplicate test --- provider/aci_test.go | 357 ------------------------------------------- 1 file changed, 357 deletions(-) diff --git a/provider/aci_test.go b/provider/aci_test.go index 7b2585e1..5b4099e1 100644 --- a/provider/aci_test.go +++ b/provider/aci_test.go @@ -1065,363 +1065,6 @@ func TestCreatePodWithReadinessProbe(t *testing.T) { } } -func TestCreatePodWithProjectedVolume(t *testing.T) { - podName := "pod-" + uuid.New().String() - podNamespace := "ns-" + uuid.New().String() - projectedVolumeName := "projectedvolume" - - aadServerMocker := NewAADMock() - aciServerMocker := NewACIMock() - - aciServerMocker.OnGetRPManifest = func() (int, interface{}) { - manifest := &aci.ResourceProviderManifest{ - Metadata: &aci.ResourceProviderMetadata{ - GPURegionalSKUs: []*aci.GPURegionalSKU{ - &aci.GPURegionalSKU{ - Location: fakeRegion, - SKUs: []aci.GPUSKU{aci.K80, aci.P100, aci.V100}, - }, - }, - }, - } - - return http.StatusOK, manifest - } - - mockCtrl := gomock.NewController(GinkgoT()) - podLister := NewMockPodLister(mockCtrl) - secretLister := NewMockSecretLister(mockCtrl) - configMapLister := NewMockConfigMapLister(mockCtrl) - serviceLister := NewMockServiceLister(mockCtrl) - pvcLister := NewMockPersistentVolumeClaimLister(mockCtrl) - pvLister := NewMockPersistentVolumeLister(mockCtrl) - - resourceManager, err := manager.NewResourceManager(podLister, secretLister, configMapLister, serviceLister, pvcLister, pvLister) - if err != nil { - t.Fatal("Unable to prepare the mocks for resourceManager", err) - } - - configMapNamespaceLister := NewMockConfigMapNamespaceLister(mockCtrl) - configMapLister.EXPECT().ConfigMaps(podNamespace).Return(configMapNamespaceLister) - configMapNamespaceLister.EXPECT().Get("kube-root-ca.crt").Return(&v1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "kube-root-ca.crt", - }, - Data: map[string]string{ - "ca.crt": "fake-ca-data", - "foo": "bar", - }, - }, nil) - - provider, err := createTestProvider(aadServerMocker, aciServerMocker, resourceManager) - if err != nil { - t.Fatal("Unable to create test provider", err) - } - encodedSecretVal := base64.StdEncoding.EncodeToString([]byte("fake-ca-data")) - - aciServerMocker.OnCreate = func(subscription, resourceGroup, containerGroup string, cg *aci.ContainerGroup) (int, interface{}) { - certVal := cg.Volumes[0].Secret["ca.crt"] - assert.Check(t, is.Equal(fakeSubscription, subscription), "Subscription doesn't match") - assert.Check(t, is.Equal(fakeResourceGroup, resourceGroup), "Resource group doesn't match") - assert.Check(t, cg != nil, "Container group is nil") - assert.Check(t, is.Equal(podNamespace+"-"+podName, containerGroup), "Container group name is not expected") - assert.Check(t, cg.ContainerGroupProperties.Containers != nil, "Containers should not be nil") - assert.Check(t, is.Equal(1, len(cg.ContainerGroupProperties.Containers)), "1 Container is expected") - assert.Check(t, is.Equal("nginx", cg.ContainerGroupProperties.Containers[0].Name), "Container nginx is expected") - assert.Check(t, is.Equal(1, len(cg.Volumes)), "volume count not match") - assert.Check(t, is.Equal(projectedVolumeName, *cg.Volumes[0].Name), "volume name doesn't match") - assert.Check(t, is.Equal(encodedSecretVal, *certVal), "configmap data doesn't match") - - return http.StatusOK, cg - } - - pod := &v1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: podName, - Namespace: podNamespace, - }, - Spec: v1.PodSpec{ - Containers: []v1.Container{ - v1.Container{ - Name: "nginx", - ReadinessProbe: &v1.Probe{ - Handler: v1.Handler{ - HTTPGet: &v1.HTTPGetAction{ - Port: intstr.FromInt(8080), - Path: "/", - }, - }, - InitialDelaySeconds: 10, - PeriodSeconds: 5, - TimeoutSeconds: 60, - SuccessThreshold: 3, - FailureThreshold: 5, - }, - }, - }, - Volumes: []v1.Volume{ - { - Name: "projectedvolume", - VolumeSource: v1.VolumeSource{ - Projected: &v1.ProjectedVolumeSource{ - Sources: []v1.VolumeProjection{ - { - ConfigMap: &v1.ConfigMapProjection{ - LocalObjectReference: v1.LocalObjectReference{ - Name: "kube-root-ca.crt", - }, - Items: []v1.KeyToPath{ - { - Key: "ca.crt", - Path: "ca.crt", - }, - }, - }, - }, - }, - }, - }, - }, - }, - }, - } - - aciServerMocker.OnGetContainerGroup = func(subscription, resourceGroup, containerGroup string) (int, interface{}) { - - assert.Check(t, is.Equal(podNamespace+"-"+podName, containerGroup), "Container group name is not expected") - - caStr := "ca.crt" - - return http.StatusOK, aci.ContainerGroup{ - Tags: map[string]string{ - "NodeName": fakeNodeName, - }, - Name: "nginx", - ContainerGroupProperties: aci.ContainerGroupProperties{ - ProvisioningState: "Creating", - Volumes: []azaci.Volume{ - { - Name: &projectedVolumeName, - Secret: map[string]*string{"Key": &caStr, "Path": &caStr}, - }, - }, - }, - } - } - - if err := provider.CreatePod(context.Background(), pod); err != nil { - t.Fatal("Failed to create pod", err) - } -} - -func TestCreatePodWithCSIVolume(t *testing.T) { - podName := "pod-name" - podNamespace := "ns-name" - fakeVolumeSecret := "fake-volume-secret" - fakeShareName := "aksshare" - azureFileVolumeName := "azure" - - aadServerMocker := NewAADMock() - aciServerMocker := NewACIMock() - mockCtrl := gomock.NewController(GinkgoT()) - mockPodLister := NewMockPodLister(mockCtrl) - mockSecretLister := NewMockSecretLister(mockCtrl) - mockSecretNamespaceLister := NewMockSecretNamespaceLister(mockCtrl) - mockConfigMapLister := NewMockConfigMapLister(mockCtrl) - mockServiceLister := NewMockServiceLister(mockCtrl) - mockPvcLister := NewMockPersistentVolumeClaimLister(mockCtrl) - mockPvLister := NewMockPersistentVolumeLister(mockCtrl) - - aciServerMocker.OnGetRPManifest = func() (int, interface{}) { - manifest := &aci.ResourceProviderManifest{ - Metadata: &aci.ResourceProviderMetadata{ - GPURegionalSKUs: []*aci.GPURegionalSKU{ - { - Location: fakeRegion, - SKUs: []aci.GPUSKU{aci.K80, aci.P100, aci.V100}, - }, - }, - }, - } - return http.StatusOK, manifest - } - - aciServerMocker.OnCreate = func(subscription, resourceGroup, containerGroup string, cg *aci.ContainerGroup) (int, interface{}) { - assert.Check(t, is.Equal(fakeSubscription, subscription), "Subscription doesn't match") - assert.Check(t, is.Equal(fakeResourceGroup, resourceGroup), "Resource group doesn't match") - assert.Check(t, cg != nil, "Container group is nil") - assert.Check(t, is.Equal(podNamespace+"-"+podName, containerGroup), "Container group name is not expected") - assert.Check(t, cg.ContainerGroupProperties.Containers != nil, "Containers should not be nil") - assert.Check(t, is.Equal(1, len(cg.ContainerGroupProperties.Containers)), "1 Container is expected") - assert.Check(t, is.Equal("nginx", cg.ContainerGroupProperties.Containers[0].Name), "Container nginx is expected") - assert.Check(t, is.Equal(1, len(cg.Volumes)), "volume count not match") - - return http.StatusOK, cg - } - - configMapNamespaceLister := NewMockConfigMapNamespaceLister(mockCtrl) - mockConfigMapLister.EXPECT().ConfigMaps(podNamespace).Return(configMapNamespaceLister) - configMapNamespaceLister.EXPECT().Get("kube-root-ca.crt").Return(&v1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "kube-root-ca.crt", - }, - Data: map[string]string{ - "ca.crt": "fake-ca-data", - "foo": "bar", - }, - }, nil) - - fakeSecret := v1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: fakeVolumeSecret, - Namespace: podNamespace, - }, - Data: map[string][]byte{ - azureFileStorageAccountName: []byte("azure storage account name"), - azureFileStorageAccountKey: []byte("azure storage account key")}, - } - - fakeVolumeMount := v1.VolumeMount{ - Name: azureFileVolumeName, - MountPath: "/mnt/azure", - } - - fakePodVolume := v1.Volume{ - Name: azureFileVolumeName, - VolumeSource: v1.VolumeSource{ - CSI: &v1.CSIVolumeSource{ - Driver: "file.csi.azure.com", - VolumeAttributes: map[string]string{ - azureFileSecretName: fakeVolumeSecret, - azureFileShareName: fakeShareName, - }, - }, - }, - } - - cases := []struct { - description string - secretVolume *v1.Secret - volume v1.Volume - expectedError error - }{ - { - description: "Secret is nil", - secretVolume: nil, - volume: fakePodVolume, - expectedError: fmt.Errorf("the secret %s for AzureFile CSI driver %s is not found", fakeSecret.Name, fakePodVolume.Name), - }, - { - description: "Volume has a secret with a valid value", - secretVolume: &fakeSecret, - volume: fakePodVolume, - expectedError: nil, - }, - { - description: "Volume has no secret", - secretVolume: &fakeSecret, - volume: v1.Volume{ - Name: azureFileVolumeName, - VolumeSource: v1.VolumeSource{ - CSI: &v1.CSIVolumeSource{ - Driver: "file.csi.azure.com", - VolumeAttributes: map[string]string{}, - }, - }}, - expectedError: fmt.Errorf("secret volume attribute for AzureFile CSI driver %s cannot be empty or nil", azureFileVolumeName), - }, - { - description: "Volume has no share name", - secretVolume: &fakeSecret, - volume: v1.Volume{ - Name: azureFileVolumeName, - VolumeSource: v1.VolumeSource{ - CSI: &v1.CSIVolumeSource{ - Driver: "file.csi.azure.com", - VolumeAttributes: map[string]string{ - azureFileSecretName: fakeVolumeSecret, - }, - }, - }}, - expectedError: fmt.Errorf("share name for AzureFile CSI driver %s cannot be empty or nil", fakePodVolume.Name), - }, - { - description: "Volume is Disk Driver", - secretVolume: &fakeSecret, - volume: v1.Volume{ - Name: azureFileVolumeName, - VolumeSource: v1.VolumeSource{ - CSI: &v1.CSIVolumeSource{ - Driver: "disk.csi.azure.com", - VolumeAttributes: map[string]string{ - azureFileSecretName: fakeVolumeSecret, - azureFileShareName: fakeShareName, - }, - }, - }}, - expectedError: fmt.Errorf("pod %s requires volume %s which is of an unsupported type %s", podName, azureFileVolumeName, "disk.csi.azure.com"), - }, - } - for _, tc := range cases { - t.Run(tc.description, func(t *testing.T) { - - resourceManager, err := manager.NewResourceManager(mockPodLister, mockSecretLister, mockConfigMapLister, mockServiceLister, mockPvcLister, mockPvLister) - if err != nil { - t.Fatal("Unable to prepare the mocks for resourceManager", err) - } - - provider, err := createTestProvider(aadServerMocker, aciServerMocker, resourceManager) - if err != nil { - t.Fatal("Unable to create test provider", err) - } - - pod := &v1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: podName, - Namespace: podNamespace, - }, - Spec: v1.PodSpec{ - Containers: []v1.Container{ - { - Name: "nginx", - ReadinessProbe: &v1.Probe{ - Handler: v1.Handler{ - HTTPGet: &v1.HTTPGetAction{ - Port: intstr.FromInt(8080), - Path: "/", - }, - }, - InitialDelaySeconds: 10, - PeriodSeconds: 5, - TimeoutSeconds: 60, - SuccessThreshold: 3, - FailureThreshold: 5, - }, - }, - }, - }, - } - - pod.Spec.Containers[0].VolumeMounts = append(pod.Spec.Containers[0].VolumeMounts, fakeVolumeMount) - - if tc.volume.CSI.VolumeAttributes != nil && len(tc.volume.CSI.VolumeAttributes) != 0 { - mockSecretLister.EXPECT().Secrets(podNamespace).Return(mockSecretNamespaceLister) - mockSecretNamespaceLister.EXPECT().Get(tc.volume.CSI.VolumeAttributes[azureFileSecretName]).Return(tc.secretVolume, nil) - } - - pod.Spec.Volumes = append(pod.Spec.Volumes, tc.volume) - - err = provider.CreatePod(context.Background(), pod) - - if tc.expectedError != nil { - assert.Equal(t, tc.expectedError.Error(), err.Error()) - } else { - assert.NilError(t, tc.expectedError, err) - } - }) - } -} - //Test create InitContainers func TestCreatePodWithInitContainer(t *testing.T) { _, aciServerMocker, provider, err := prepareMocks() From 007f7438c3329ca2f6791324fbb33dd8b98e1712 Mon Sep 17 00:00:00 2001 From: Arnav Arnav Date: Mon, 31 Oct 2022 11:03:41 -0700 Subject: [PATCH 34/46] removed init containers from old code before sdk --- client/aci/types.go | 16 ------ provider/aci.go | 121 +++++++++----------------------------------- 2 files changed, 24 insertions(+), 113 deletions(-) diff --git a/client/aci/types.go b/client/aci/types.go index b14f4a18..f4c860fa 100644 --- a/client/aci/types.go +++ b/client/aci/types.go @@ -65,21 +65,6 @@ type Container struct { ContainerProperties `json:"properties,omitempty"` } -// InitContainerDefinition is a initContainer instance. -type InitContainerDefinition struct { - Name string `json:"name,omitempty"` - InitContainerProperties `json:"properties,omitempty"` -} - -// InitContainerProperties is the initContainer instance properties. -type InitContainerProperties struct { - Image string `json:"image,omitempty"` - Command []string `json:"command,omitempty"` - EnvironmentVariables []EnvironmentVariable `json:"environmentVariables,omitempty"` - InstanceView ContainerPropertiesInstanceView `json:"instanceView,omitempty"` - VolumeMounts []VolumeMount `json:"volumeMounts,omitempty"` -} - // ContainerGroup is a container group. type ContainerGroup struct { api.ResponseMetadata `json:"-"` @@ -95,7 +80,6 @@ type ContainerGroup struct { type ContainerGroupProperties struct { ProvisioningState string `json:"provisioningState,omitempty"` Containers []Container `json:"containers,omitempty"` - InitContainers []InitContainerDefinition `json:"initContainers,omitempty"` ImageRegistryCredentials []ImageRegistryCredential `json:"imageRegistryCredentials,omitempty"` RestartPolicy ContainerGroupRestartPolicy `json:"restartPolicy,omitempty"` IPAddress *IPAddress `json:"ipAddress,omitempty"` diff --git a/provider/aci.go b/provider/aci.go index 8cdf5b0c..649055f8 100644 --- a/provider/aci.go +++ b/provider/aci.go @@ -680,11 +680,6 @@ func (p *ACIProvider) CreatePod(ctx context.Context, pod *v1.Pod) error { containerGroup.RestartPolicy = aci.ContainerGroupRestartPolicy(pod.Spec.RestartPolicy) containerGroup.ContainerGroupProperties.OsType = aci.OperatingSystemTypes(p.operatingSystem) - // get initContainers - initContainers, err := p.getInitContainers(pod) - if err != nil { - return err - } // get containers containers, err := p.getContainers(pod) if err != nil { @@ -701,7 +696,6 @@ func (p *ACIProvider) CreatePod(ctx context.Context, pod *v1.Pod) error { return err } // assign all the things - containerGroup.ContainerGroupProperties.InitContainers = initContainers containerGroup.ContainerGroupProperties.Containers = containers containerGroup.ContainerGroupProperties.Volumes = volumes containerGroup.ContainerGroupProperties.ImageRegistryCredentials = creds @@ -1467,103 +1461,22 @@ func readDockerConfigJSONSecret(secret *v1.Secret, ips []aci.ImageRegistryCreden return ips, err } -//verify if Container is properly declared for the use on ACI -//this method is used for both initConainers and containers -func (p *ACIProvider) verifyContainer(container *v1.Container) error { - if len(container.Command) == 0 && len(container.Args) > 0 { - return errdefs.InvalidInput("ACI does not support providing args without specifying the command. Please supply both command and args to the pod spec.") - } - return nil -} - -//get the command declared on Container -//this method is used for both initConainers and containers -func (p *ACIProvider) getCommand(container *v1.Container) []string { - return append(container.Command, container.Args...) -} - -//get VolumeMounts declared on Container as []aci.VolumeMount -//this method is used for both initConainers and containers -func (p *ACIProvider) getVolumeMounts(container *v1.Container) []aci.VolumeMount { - volumeMounts := make([]aci.VolumeMount, 0, len(container.VolumeMounts)) - for _, v := range container.VolumeMounts { - volumeMounts = append(volumeMounts, aci.VolumeMount{ - Name: v.Name, - MountPath: v.MountPath, - ReadOnly: v.ReadOnly, - }) - } - return volumeMounts -} - -//get EnvironmentVariables declared on Container as []aci.EnvironmentVariable -//this method is used for both initConainers and containers -func (p *ACIProvider) getEnvironmentVariables(container *v1.Container) []aci.EnvironmentVariable { - environmentVariable := make([]aci.EnvironmentVariable, 0, len(container.Env)) - for _, e := range container.Env { - if e.Value != "" { - envVar := getACIEnvVar(e) - environmentVariable = append(environmentVariable, envVar) - } - } - return environmentVariable -} - -//get InitContainers defined in Pod as []aci.InitContainerDefinition -func (p *ACIProvider) getInitContainers(pod *v1.Pod) ([]aci.InitContainerDefinition, error) { - initContainers := make([]aci.InitContainerDefinition, 0, len(pod.Spec.InitContainers)) - for _, initContainer := range pod.Spec.InitContainers { - err := p.verifyContainer(&initContainer) - if err != nil { - return nil, err - } - - if initContainer.Ports != nil { - return nil, errdefs.InvalidInput("ACI initContainers does not support ports.") - } - if initContainer.Resources.Requests != nil { - return nil, errdefs.InvalidInput("ACI initContainers does not support resources requests.") - } - if initContainer.Resources.Limits != nil { - return nil, errdefs.InvalidInput("ACI initContainers does not support resources limits.") - } - if initContainer.LivenessProbe != nil { - return nil, errdefs.InvalidInput("ACI initContainers does not support livenessProbe.") - } - if initContainer.ReadinessProbe != nil { - return nil, errdefs.InvalidInput("ACI initContainers does not support readinessProbe.") - } - - newInitContainer := aci.InitContainerDefinition{ - Name: initContainer.Name, - } - newInitContainer.Image = initContainer.Image - newInitContainer.Command = p.getCommand(&initContainer) - - newInitContainer.VolumeMounts = p.getVolumeMounts(&initContainer) - newInitContainer.EnvironmentVariables = p.getEnvironmentVariables(&initContainer) - - initContainers = append(initContainers, newInitContainer) - } - return initContainers, nil -} - -//get Containers defined in Pod as []aci.Container type func (p *ACIProvider) getContainers(pod *v1.Pod) ([]aci.Container, error) { containers := make([]aci.Container, 0, len(pod.Spec.Containers)) for _, container := range pod.Spec.Containers { - err := p.verifyContainer(&container) - if err != nil { - return nil, err - } + if len(container.Command) == 0 && len(container.Args) > 0 { + return nil, errdefs.InvalidInput("ACI does not support providing args without specifying the command. Please supply both command and args to the pod spec.") + } c := aci.Container{ Name: container.Name, + ContainerProperties: aci.ContainerProperties{ + Image: container.Image, + Command: append(container.Command, container.Args...), + Ports: make([]aci.ContainerPort, 0, len(container.Ports)), + }, } - c.Image = container.Image - c.Command = p.getCommand(&container) - c.Ports = make([]aci.ContainerPort, 0, len(container.Ports)) for _, p := range container.Ports { c.Ports = append(c.Ports, aci.ContainerPort{ Port: p.ContainerPort, @@ -1571,8 +1484,22 @@ func (p *ACIProvider) getContainers(pod *v1.Pod) ([]aci.Container, error) { }) } - c.VolumeMounts = p.getVolumeMounts(&container) - c.EnvironmentVariables = p.getEnvironmentVariables(&container) + c.VolumeMounts = make([]aci.VolumeMount, 0, len(container.VolumeMounts)) + for _, v := range container.VolumeMounts { + c.VolumeMounts = append(c.VolumeMounts, aci.VolumeMount{ + Name: v.Name, + MountPath: v.MountPath, + ReadOnly: v.ReadOnly, + }) + } + + c.EnvironmentVariables = make([]aci.EnvironmentVariable, 0, len(container.Env)) + for _, e := range container.Env { + if e.Value != "" { + envVar := getACIEnvVar(e) + c.EnvironmentVariables = append(c.EnvironmentVariables, envVar) + } + } // NOTE(robbiezhang): ACI CPU request must be times of 10m cpuRequest := 1.00 From b60194b7b1f92f78a91fe23be334b1d92c0486ee Mon Sep 17 00:00:00 2001 From: Arnav Arnav Date: Mon, 31 Oct 2022 11:05:16 -0700 Subject: [PATCH 35/46] removed init container test from old code before sdk --- provider/aci_test.go | 330 ------------------------------------------- 1 file changed, 330 deletions(-) diff --git a/provider/aci_test.go b/provider/aci_test.go index 5b4099e1..561846a3 100644 --- a/provider/aci_test.go +++ b/provider/aci_test.go @@ -1064,333 +1064,3 @@ func TestCreatePodWithReadinessProbe(t *testing.T) { t.Fatal("Failed to create pod", err) } } - -//Test create InitContainers -func TestCreatePodWithInitContainer(t *testing.T) { - _, aciServerMocker, provider, err := prepareMocks() - - if err != nil { - t.Fatal("Unable to prepare the mocks", err) - } - - podName := "pod-" + uuid.New().String() - podNamespace := "ns-" + uuid.New().String() - azureFileVolumeName := "azure" - - aciServerMocker.OnCreate = func(subscription, resourceGroup, containerGroup string, cg *aci.ContainerGroup) (int, interface{}) { - assert.Check(t, is.Equal(fakeSubscription, subscription), "Subscription doesn't match") - assert.Check(t, is.Equal(fakeResourceGroup, resourceGroup), "Resource group doesn't match") - - assert.Check(t, cg != nil, "Container group is nil") - assert.Check(t, cg.ContainerGroupProperties.Containers != nil, "Containers should not be nil") - assert.Check(t, cg.ContainerGroupProperties.InitContainers != nil, "InitContainer should not be nil") - assert.Check(t, is.Equal(1, len(cg.ContainerGroupProperties.Containers)), "1 Container is expected") - assert.Check(t, is.Equal(2, len(cg.ContainerGroupProperties.InitContainers)), "2 InitContainers are expected") - - assert.Check(t, is.Equal("initContainer 01", cg.ContainerGroupProperties.InitContainers[0].Name), "first initContainer name must be \"initContainer 01\"") - assert.Check(t, is.Equal("alpine", cg.ContainerGroupProperties.InitContainers[0].Image), "InitContainer Image is not the expected") - assert.Check(t, cg.ContainerGroupProperties.InitContainers[0].VolumeMounts != nil, "VolumeMounts for InitContainer should not be nil") - - return http.StatusOK, cg - } - - fakeVolumeMount := v1.VolumeMount{ - Name: azureFileVolumeName, - MountPath: "/mnt/azure", - } - - pod := &v1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: podName, - Namespace: podNamespace, - }, - Spec: v1.PodSpec{ - Containers: []v1.Container{ - { - Name: "container name", - }, - }, - InitContainers: []v1.Container{ - { - Name: "initContainer 01", - Image: "alpine", - }, - { - Name: "initContainer 02", - Image: "alpine", - }, - }, - }, - } - pod.Spec.InitContainers[0].VolumeMounts = append(pod.Spec.InitContainers[0].VolumeMounts, fakeVolumeMount) - - if err := provider.CreatePod(context.Background(), pod); err != nil { - t.Fatal("Failed to create pod", err) - } -} - -//Test Init Container with Ports, -//ACI does not support initContainer with ports, errors expected -func TestCreateInitContainerWithPort(t *testing.T) { - _, aciServerMocker, provider, err := prepareMocks() - - if err != nil { - t.Fatal("Unable to prepare the mocks", err) - } - - podName := "pod-" + uuid.New().String() - podNamespace := "ns-" + uuid.New().String() - - aciServerMocker.OnCreate = func(subscription, resourceGroup, containerGroup string, cg *aci.ContainerGroup) (int, interface{}) { - assert.Check(t, is.Equal(fakeSubscription, subscription), "Subscription doesn't match") - assert.Check(t, is.Equal(fakeResourceGroup, resourceGroup), "Resource group doesn't match") - return http.StatusOK, cg - } - - pod := &v1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: podName, - Namespace: podNamespace, - }, - Spec: v1.PodSpec{ - Containers: []v1.Container{ - { - Name: "container name", - }, - }, - InitContainers: []v1.Container{ - { - Name: "initContainer 01", - Ports: []v1.ContainerPort{ - { - Name: "port name", - ContainerPort: 80, - }, - }, - }, - }, - }, - } - - if err := provider.CreatePod(context.Background(), pod); err == nil { - t.Fatal("Ports for ACI initContainers are not supported", err) - } -} - -//Test Init Container with Resources specs, -//ACI does not support initContainer with Resources specs, errors expected -func TestCreateInitContainerWithResourcesSpecs(t *testing.T) { - _, aciServerMocker, provider, err := prepareMocks() - - if err != nil { - t.Fatal("Unable to prepare the mocks", err) - } - - podName := "pod-" + uuid.New().String() - podNamespace := "ns-" + uuid.New().String() - - aciServerMocker.OnCreate = func(subscription, resourceGroup, containerGroup string, cg *aci.ContainerGroup) (int, interface{}) { - assert.Check(t, is.Equal(fakeSubscription, subscription), "Subscription doesn't match") - assert.Check(t, is.Equal(fakeResourceGroup, resourceGroup), "Resource group doesn't match") - return http.StatusOK, cg - } - - pod := &v1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: podName, - Namespace: podNamespace, - }, - Spec: v1.PodSpec{ - Containers: []v1.Container{ - { - Name: "container name", - }, - }, - InitContainers: []v1.Container{ - { - Name: "initContainer 01", - Resources: v1.ResourceRequirements{ - Requests: v1.ResourceList{ - "cpu": resource.MustParse("1.981"), - "memory": resource.MustParse("3.49G"), - }, - Limits: v1.ResourceList{ - gpuResourceName: resource.MustParse("10"), - }, - }, - }, - }, - }, - } - - if err := provider.CreatePod(context.Background(), pod); err == nil { - t.Fatal("Resource specs for ACI initContainers are not supported", err) - } - - pod = &v1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: podName, - Namespace: podNamespace, - }, - Spec: v1.PodSpec{ - Containers: []v1.Container{ - { - Name: "container name", - }, - }, - InitContainers: []v1.Container{ - { - Name: "initContainer 01", - Resources: v1.ResourceRequirements{ - Limits: v1.ResourceList{ - gpuResourceName: resource.MustParse("10"), - }, - }, - }, - }, - }, - } - - if err := provider.CreatePod(context.Background(), pod); err == nil { - t.Fatal("Resource specs for ACI initContainers are not supported", err) - } - - pod = &v1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: podName, - Namespace: podNamespace, - }, - Spec: v1.PodSpec{ - Containers: []v1.Container{ - { - Name: "container name", - }, - }, - InitContainers: []v1.Container{ - { - Name: "initContainer 01", - Resources: v1.ResourceRequirements{ - Requests: v1.ResourceList{ - "cpu": resource.MustParse("1.981"), - "memory": resource.MustParse("3.49G"), - }, - }, - }, - }, - }, - } - - if err := provider.CreatePod(context.Background(), pod); err == nil { - t.Fatal("Resource specs for ACI initContainers are not supported", err) - } -} - -//Test Init Container with LivenessProbe, -//ACI does not support initContainer with LivenessProbe, errors expected -func TestCreateInitContainerWithLivenessProbe(t *testing.T) { - _, aciServerMocker, provider, err := prepareMocks() - - if err != nil { - t.Fatal("Unable to prepare the mocks", err) - } - - podName := "pod-" + uuid.New().String() - podNamespace := "ns-" + uuid.New().String() - - aciServerMocker.OnCreate = func(subscription, resourceGroup, containerGroup string, cg *aci.ContainerGroup) (int, interface{}) { - assert.Check(t, is.Equal(fakeSubscription, subscription), "Subscription doesn't match") - assert.Check(t, is.Equal(fakeResourceGroup, resourceGroup), "Resource group doesn't match") - return http.StatusOK, cg - } - - pod := &v1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: podName, - Namespace: podNamespace, - }, - Spec: v1.PodSpec{ - Containers: []v1.Container{ - { - Name: "container name", - }, - }, - InitContainers: []v1.Container{ - { - Name: "initContainer 01", - LivenessProbe: &v1.Probe{ - Handler: v1.Handler{ - HTTPGet: &v1.HTTPGetAction{ - Port: intstr.FromString("http"), - Path: "/", - }, - }, - InitialDelaySeconds: 10, - PeriodSeconds: 5, - TimeoutSeconds: 60, - SuccessThreshold: 3, - FailureThreshold: 5, - }, - }, - }, - }, - } - - if err := provider.CreatePod(context.Background(), pod); err == nil { - t.Fatal("LivenessProbe for ACI initContainers is not supported", err) - } -} - -//Test Init Container with ReadinessProbe, -//ACI does not support initContainer with ReadinessProbe, errors expected -func TestCreateInitContainerWithReadinessProbe(t *testing.T) { - _, aciServerMocker, provider, err := prepareMocks() - - if err != nil { - t.Fatal("Unable to prepare the mocks", err) - } - - podName := "pod-" + uuid.New().String() - podNamespace := "ns-" + uuid.New().String() - - aciServerMocker.OnCreate = func(subscription, resourceGroup, containerGroup string, cg *aci.ContainerGroup) (int, interface{}) { - assert.Check(t, is.Equal(fakeSubscription, subscription), "Subscription doesn't match") - assert.Check(t, is.Equal(fakeResourceGroup, resourceGroup), "Resource group doesn't match") - return http.StatusOK, cg - } - - pod := &v1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: podName, - Namespace: podNamespace, - }, - Spec: v1.PodSpec{ - Containers: []v1.Container{ - { - Name: "container name", - }, - }, - InitContainers: []v1.Container{ - { - Name: "initContainer 01", - ReadinessProbe: &v1.Probe{ - Handler: v1.Handler{ - HTTPGet: &v1.HTTPGetAction{ - Port: intstr.FromInt(8080), - Path: "/", - }, - }, - InitialDelaySeconds: 10, - PeriodSeconds: 5, - TimeoutSeconds: 60, - SuccessThreshold: 3, - FailureThreshold: 5, - }, - }, - }, - }, - } - - if err := provider.CreatePod(context.Background(), pod); err == nil { - t.Fatal("ReadinessProbe for ACI initContainers is not supported", err) - } -} From 186c22035032ba7ef2e66885ab9ec079bd76f35a Mon Sep 17 00:00:00 2001 From: Arnav Arnav Date: Mon, 31 Oct 2022 11:12:57 -0700 Subject: [PATCH 36/46] removed unused import --- pkg/provider/aci_init_container_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/provider/aci_init_container_test.go b/pkg/provider/aci_init_container_test.go index ab306029..889985e0 100644 --- a/pkg/provider/aci_init_container_test.go +++ b/pkg/provider/aci_init_container_test.go @@ -3,7 +3,6 @@ package provider import ( "context" - "fmt" "testing" "github.com/virtual-kubelet/virtual-kubelet/errdefs" From 899e83b87b0c103bc725077a990641c9bbb4f76c Mon Sep 17 00:00:00 2001 From: Arnav Arnav Date: Mon, 31 Oct 2022 17:47:26 -0700 Subject: [PATCH 37/46] fix error with pointer --- pkg/provider/aci.go | 6 +++--- pkg/provider/aci_init_container_test.go | 8 ++++++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/pkg/provider/aci.go b/pkg/provider/aci.go index 3847a038..78f25e74 100644 --- a/pkg/provider/aci.go +++ b/pkg/provider/aci.go @@ -1263,7 +1263,7 @@ func (p *ACIProvider) getEnvironmentVariables(container *v1.Container) *[]azaci. //get InitContainers defined in Pod as []aci.InitContainerDefinition func (p *ACIProvider) getInitContainers(pod *v1.Pod) ([]azaci.InitContainerDefinition, error) { initContainers := make([]azaci.InitContainerDefinition, 0, len(pod.Spec.InitContainers)) - for _, initContainer := range pod.Spec.InitContainers { + for i, initContainer := range pod.Spec.InitContainers { err := p.verifyContainer(&initContainer) if err != nil { return nil, err @@ -1286,9 +1286,9 @@ func (p *ACIProvider) getInitContainers(pod *v1.Pod) ([]azaci.InitContainerDefin } newInitContainer := azaci.InitContainerDefinition{ - Name: &initContainer.Name, + Name: &pod.Spec.InitContainers[i].Name, InitContainerPropertiesDefinition: &azaci.InitContainerPropertiesDefinition { - Image: &initContainer.Image, + Image: &pod.Spec.InitContainers[i].Image, Command: p.getCommand(&initContainer), VolumeMounts: p.getVolumeMounts(&initContainer), EnvironmentVariables: p.getEnvironmentVariables(&initContainer), diff --git a/pkg/provider/aci_init_container_test.go b/pkg/provider/aci_init_container_test.go index 889985e0..72aa00f0 100644 --- a/pkg/provider/aci_init_container_test.go +++ b/pkg/provider/aci_init_container_test.go @@ -19,6 +19,8 @@ import ( func TestCreatePodWithInitContainers(t *testing.T) { + initContainerName1 := "init-container-1" + initContainerName2 := "init-container-2" mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() @@ -35,6 +37,8 @@ func TestCreatePodWithInitContainers(t *testing.T) { assert.Check(t, initContainers[0].EnvironmentVariables != nil, "Volume mount should be present") assert.Check(t, initContainers[0].Command != nil, "Command mount should be present") assert.Check(t, initContainers[0].Image != nil, "Image should be present") + assert.Check(t, *initContainers[0].Name == initContainerName1, "Name should be correct") + assert.Check(t, *initContainers[1].Name == initContainerName2, "Name should be correct") return nil } @@ -63,7 +67,7 @@ func TestCreatePodWithInitContainers(t *testing.T) { expectedError: nil, initContainers: []v1.Container{ v1.Container{ - Name: "initContainer 01", + Name: initContainerName1, Image: "alpine", VolumeMounts: []v1.VolumeMount{ v1.VolumeMount{ @@ -81,7 +85,7 @@ func TestCreatePodWithInitContainers(t *testing.T) { }, }, v1.Container{ - Name: "initContainer 02", + Name: initContainerName2, Image: "alpine", }, }, From 2b53bf4dc26f650e07f329387001a409989c36fd Mon Sep 17 00:00:00 2001 From: Arnav Arnav Date: Wed, 2 Nov 2022 11:48:37 -0700 Subject: [PATCH 38/46] added initContainer to AzureFilesVolume test; log errors; error text formatting --- pkg/provider/aci.go | 20 +++++++++++------- pkg/provider/aci_init_container_test.go | 17 ++++++++++------ pkg/provider/aci_volumes_test.go | 27 +++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 13 deletions(-) diff --git a/pkg/provider/aci.go b/pkg/provider/aci.go index 78f25e74..d4878c2b 100644 --- a/pkg/provider/aci.go +++ b/pkg/provider/aci.go @@ -486,7 +486,7 @@ func (p *ACIProvider) CreatePod(ctx context.Context, pod *v1.Pod) error { } // get initContainers - initContainers, err := p.getInitContainers(pod) + initContainers, err := p.getInitContainers(ctx, pod) if err != nil { return err } @@ -1261,28 +1261,34 @@ func (p *ACIProvider) getEnvironmentVariables(container *v1.Container) *[]azaci. } //get InitContainers defined in Pod as []aci.InitContainerDefinition -func (p *ACIProvider) getInitContainers(pod *v1.Pod) ([]azaci.InitContainerDefinition, error) { +func (p *ACIProvider) getInitContainers(ctx context.Context, pod *v1.Pod) ([]azaci.InitContainerDefinition, error) { initContainers := make([]azaci.InitContainerDefinition, 0, len(pod.Spec.InitContainers)) for i, initContainer := range pod.Spec.InitContainers { err := p.verifyContainer(&initContainer) if err != nil { + log.G(ctx).Errorf("couldn't verify container %v", err) return nil, err } if initContainer.Ports != nil { - return nil, errdefs.InvalidInput("ACI initContainers does not support ports.") + log.G(ctx).Errorf("azure container instances initcontainers do not support ports") + return nil, errdefs.InvalidInput("azure container instances initContainers do not support ports") } if initContainer.Resources.Requests != nil { - return nil, errdefs.InvalidInput("ACI initContainers does not support resources requests.") + log.G(ctx).Errorf("azure container instances initcontainers do not support resources requests") + return nil, errdefs.InvalidInput("azure container instances initContainers do not support resources requests") } if initContainer.Resources.Limits != nil { - return nil, errdefs.InvalidInput("ACI initContainers does not support resources limits.") + log.G(ctx).Errorf("azure container instances initcontainers do not support resources limits") + return nil, errdefs.InvalidInput("azure container instances initContainers do not support resources limits") } if initContainer.LivenessProbe != nil { - return nil, errdefs.InvalidInput("ACI initContainers does not support livenessProbe.") + log.G(ctx).Errorf("azure container instances initcontainers do not support livenessProbe") + return nil, errdefs.InvalidInput("azure container instances initContainers do not support livenessProbe") } if initContainer.ReadinessProbe != nil { - return nil, errdefs.InvalidInput("ACI initContainers does not support readinessProbe.") + log.G(ctx).Errorf("azure container instances initcontainers do not support readinessProbe") + return nil, errdefs.InvalidInput("azure container instances initContainers do not support readinessProbe") } newInitContainer := azaci.InitContainerDefinition{ diff --git a/pkg/provider/aci_init_container_test.go b/pkg/provider/aci_init_container_test.go index 72aa00f0..e9de3d13 100644 --- a/pkg/provider/aci_init_container_test.go +++ b/pkg/provider/aci_init_container_test.go @@ -31,7 +31,7 @@ func TestCreatePodWithInitContainers(t *testing.T) { 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") - assert.Check(t, is.Equal(len(containers), 1), "1 Container is expected") + assert.Check(t, is.Equal(len(containers), 2), "1 Container is expected") assert.Check(t, is.Equal(len(initContainers), 2), "2 init containers are expected") assert.Check(t, initContainers[0].VolumeMounts != nil, "Volume mount should be present") assert.Check(t, initContainers[0].EnvironmentVariables != nil, "Volume mount should be present") @@ -52,7 +52,12 @@ func TestCreatePodWithInitContainers(t *testing.T) { Spec: v1.PodSpec{ Containers: []v1.Container{ { - Name: "container name", + Name: "container-name-01", + Image: "alpine", + }, + { + Name: "container-name-02", + Image: "alpine", }, }, }, @@ -105,7 +110,7 @@ func TestCreatePodWithInitContainers(t *testing.T) { }, }, }, - expectedError: errdefs.InvalidInput("ACI initContainers does not support ports."), + expectedError: errdefs.InvalidInput("azure container instances initContainers do not support ports"), }, { description: "Init Containers with liveness probe", @@ -127,7 +132,7 @@ func TestCreatePodWithInitContainers(t *testing.T) { }, }, }, - expectedError: errdefs.InvalidInput("ACI initContainers does not support livenessProbe."), + expectedError: errdefs.InvalidInput("azure container instances initContainers do not support livenessProbe"), }, { description: "Init Containers with readiness probe", @@ -149,7 +154,7 @@ func TestCreatePodWithInitContainers(t *testing.T) { }, }, }, - expectedError: errdefs.InvalidInput("ACI initContainers does not support readinessProbe."), + expectedError: errdefs.InvalidInput("azure container instances initContainers do not support readinessProbe"), }, { description: "Init Containers with resource request", @@ -167,7 +172,7 @@ func TestCreatePodWithInitContainers(t *testing.T) { }, }, }, - expectedError: errdefs.InvalidInput("ACI initContainers does not support resources requests."), + expectedError: errdefs.InvalidInput("azure container instances initContainers do not support resources requests"), }, } for _, tc := range cases { diff --git a/pkg/provider/aci_volumes_test.go b/pkg/provider/aci_volumes_test.go index dff0c455..8c465af7 100644 --- a/pkg/provider/aci_volumes_test.go +++ b/pkg/provider/aci_volumes_test.go @@ -30,6 +30,7 @@ func TestCreatedPodWithAzureFilesVolume(t *testing.T) { azureFileVolumeName1 := "azurefile1" azureFileVolumeName2 := "azurefile2" fakeSecretName := "fake-secret" + initContainerName := "init-container" mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() @@ -37,6 +38,7 @@ func TestCreatedPodWithAzureFilesVolume(t *testing.T) { aciMocks := createNewACIMock() aciMocks.MockCreateContainerGroup = func(ctx context.Context, resourceGroup, podNS, podName string, cg *client.ContainerGroupWrapper) error { containers := *cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Containers + initContainers := *cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.InitContainers 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") @@ -46,6 +48,11 @@ func TestCreatedPodWithAzureFilesVolume(t *testing.T) { assert.Check(t, is.Equal(fakeShareName1, *(*cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Volumes)[1].AzureFile.ShareName), "volume share name is not matched") assert.Check(t, is.Equal(azureFileVolumeName2, *(*cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Volumes)[2].Name), "volume name is not matched") assert.Check(t, is.Equal(fakeShareName2, *(*cg.ContainerGroupPropertiesWrapper.ContainerGroupProperties.Volumes)[2].AzureFile.ShareName), "volume share name is not matched") + assert.Check(t, initContainers[0].VolumeMounts != nil, "Volume mount should be present") + assert.Check(t, initContainers[0].EnvironmentVariables != nil, "Volume mount should be present") + assert.Check(t, initContainers[0].Command != nil, "Command mount should be present") + assert.Check(t, initContainers[0].Image != nil, "Image should be present") + assert.Check(t, *initContainers[0].Name == initContainerName, "Name should be correct") return nil } @@ -163,6 +170,26 @@ func TestCreatedPodWithAzureFilesVolume(t *testing.T) { }, }, }, + InitContainers: []v1.Container{ + v1.Container{ + Name: initContainerName, + Image: "alpine", + VolumeMounts: []v1.VolumeMount{ + v1.VolumeMount{ + Name: "fakeVolumeName", + MountPath: "/mnt/azure", + }, + }, + Command: []string{"/bin/bash"}, + Args: []string{"-c echo test"}, + Env: []v1.EnvVar{ + v1.EnvVar{ + Name: "TEST_ENV", + Value: "testvalue", + }, + }, + }, + }, }, } From 0f214fc9ede4426334b263313364130b3296df63 Mon Sep 17 00:00:00 2001 From: Arnav Arnav Date: Wed, 2 Nov 2022 11:51:29 -0700 Subject: [PATCH 39/46] removed init containers from limitations --- README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/README.md b/README.md index ead4dff6..63200a3b 100644 --- a/README.md +++ b/README.md @@ -45,7 +45,6 @@ Virtual Kubelet's ACI provider relies heavily on the feature set that Azure Cont * [Limitations](https://docs.microsoft.com/azure/container-instances/container-instances-vnet) with VNet * VNet peering * Argument support for exec -* Init containers * [Host aliases](https://kubernetes.io/docs/concepts/services-networking/add-entries-to-pod-etc-hosts-with-host-aliases/) support * downward APIs (i.e podIP) From 79920d2f7af4101d0df8875a65103878ffd9bd20 Mon Sep 17 00:00:00 2001 From: Arnav Arnav Date: Wed, 2 Nov 2022 16:45:06 -0700 Subject: [PATCH 40/46] added volume to init container e2e test --- e2e/fixtures/initcontainers_pod.yml | 10 ++++++++++ e2e/pods_test.go | 5 ++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/e2e/fixtures/initcontainers_pod.yml b/e2e/fixtures/initcontainers_pod.yml index 2878aed1..c136ecaa 100644 --- a/e2e/fixtures/initcontainers_pod.yml +++ b/e2e/fixtures/initcontainers_pod.yml @@ -26,6 +26,9 @@ spec: requests: memory: 1G cpu: 1 + volumeMounts: + - name: azure + mountPath: /mnt/azure nodeSelector: kubernetes.io/role: agent beta.kubernetes.io/os: linux @@ -33,3 +36,10 @@ spec: tolerations: - key: virtual-kubelet.io/provider operator: Exists + volumes: + - name: azure + csi: + driver: file.csi.azure.com + volumeAttributes: + secretName: csidriversecret # required + shareName: vncsidriversharename # required diff --git a/e2e/pods_test.go b/e2e/pods_test.go index 8c664b18..2071422f 100644 --- a/e2e/pods_test.go +++ b/e2e/pods_test.go @@ -71,7 +71,10 @@ func TestPodWithInitContainers(t *testing.T) { t.Fatal(string(out)) } - cmd = kubectl("apply", "-f", "fixtures/initcontainers_pod.yml", "--namespace=vk-test") + testStorageAccount := os.Getenv("CSI_DRIVER_STORAGE_ACCOUNT_NAME") + testStorageKey := os.Getenv("CSI_DRIVER_STORAGE_ACCOUNT_KEY") + + cmd = kubectl("create", "secret", "generic", "csidriversecret", "--from-literal", "azurestorageaccountname="+testStorageAccount, "--from-literal", "azurestorageaccountkey="+testStorageKey, "--namespace=vk-test") if out, err := cmd.CombinedOutput(); err != nil { t.Fatal(string(out)) } From ee2d54dbc99e6576c1667d15cef4a12c452481c8 Mon Sep 17 00:00:00 2001 From: Arnav Arnav Date: Wed, 9 Nov 2022 11:27:05 -0800 Subject: [PATCH 41/46] create pod correctly in e2e test --- e2e/pods_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/e2e/pods_test.go b/e2e/pods_test.go index 2071422f..1dbdcb7f 100644 --- a/e2e/pods_test.go +++ b/e2e/pods_test.go @@ -79,6 +79,10 @@ func TestPodWithInitContainers(t *testing.T) { t.Fatal(string(out)) } + cmd = kubectl("apply", "-f", "fixtures/initcontainers_pod.yml") + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatal(string(out)) + } deadline, ok := t.Deadline() timeout := time.Until(deadline) if !ok { From a75b217a1d20fd9bd008480f691076dcc4539703 Mon Sep 17 00:00:00 2001 From: Arnav Arnav Date: Mon, 14 Nov 2022 14:39:59 -0800 Subject: [PATCH 42/46] added e2e test for init container order --- e2e/fixtures/initcontainers_ordertest_pod.yml | 44 +++++++++++++ e2e/pods_test.go | 65 +++++++++++++++++++ pkg/provider/aci.go | 15 +++-- 3 files changed, 117 insertions(+), 7 deletions(-) create mode 100644 e2e/fixtures/initcontainers_ordertest_pod.yml diff --git a/e2e/fixtures/initcontainers_ordertest_pod.yml b/e2e/fixtures/initcontainers_ordertest_pod.yml new file mode 100644 index 00000000..11ed8c27 --- /dev/null +++ b/e2e/fixtures/initcontainers_ordertest_pod.yml @@ -0,0 +1,44 @@ +apiVersion: v1 +kind: Pod +metadata: + name: vk-e2e-initcontainers-order + namespace: vk-test +spec: + initContainers: + - image: alpine + name: init-container-01 + command: [ "/bin/sh" ] + args: [ "-c", "echo Hi from init-container-01 >> /mnt/azure/newfile.txt" ] + volumeMounts: + - name: azure + mountPath: /mnt/azure + containers: + - image: alpine + imagePullPolicy: Always + name: container + command: [ + "sh", + "-c", + "echo Hi from container >> /mnt/azure/newfile.txt; while sleep 10; do cat /mnt/azure/newfile.txt; done;" + ] + resources: + requests: + memory: 1G + cpu: 1 + volumeMounts: + - name: azure + mountPath: /mnt/azure + nodeSelector: + kubernetes.io/role: agent + beta.kubernetes.io/os: linux + type: virtual-kubelet + tolerations: + - key: virtual-kubelet.io/provider + operator: Exists + volumes: + - name: azure + csi: + driver: file.csi.azure.com + volumeAttributes: + secretName: csidriversecret # required + shareName: vncsidriversharename # required diff --git a/e2e/pods_test.go b/e2e/pods_test.go index 1dbdcb7f..b6b09be2 100644 --- a/e2e/pods_test.go +++ b/e2e/pods_test.go @@ -4,6 +4,10 @@ import ( "os" "testing" "time" + "io/ioutil" + "os/exec" + + "gotest.tools/assert" ) func TestPodLifecycle(t *testing.T) { @@ -116,6 +120,67 @@ func TestPodWithInitContainers(t *testing.T) { } } +func TestPodWithInitContainersOrder(t *testing.T) { + // delete the namespace first + cmd := kubectl("delete", "namespace", "vk-test", "--ignore-not-found") + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatal(string(out)) + } + + // create namespace + cmd = kubectl("apply", "-f", "fixtures/namespace.yml") + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatal(string(out)) + } + + testStorageAccount := os.Getenv("CSI_DRIVER_STORAGE_ACCOUNT_NAME") + testStorageKey := os.Getenv("CSI_DRIVER_STORAGE_ACCOUNT_KEY") + + cmd = kubectl("create", "secret", "generic", "csidriversecret", "--from-literal", "azurestorageaccountname="+testStorageAccount, "--from-literal", "azurestorageaccountkey="+testStorageKey, "--namespace=vk-test") + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatal(string(out)) + } + + cmd = kubectl("apply", "-f", "fixtures/initcontainers_ordertest_pod.yml") + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatal(string(out)) + } + deadline, ok := t.Deadline() + timeout := time.Until(deadline) + if !ok { + timeout = 300 * time.Second + } + cmd = kubectl("wait", "--for=condition=ready", "--timeout="+timeout.String(), "pod/vk-e2e-initcontainers-order", "--namespace=vk-test") + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatal(string(out)) + } + t.Log("success create pod") + + // download file created by pod + cmd = exec.Command("az", "storage", "file", "download", "--account-name", testStorageAccount, "--account-key", testStorageKey, "-s", "vncsidriversharename", "-p", "newfile.txt") + cmd.Env = os.Environ() + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatal(string(out)) + } + t.Log("file newfile.txt downloaded from storage account") + + file, err := ioutil.ReadFile("newfile.txt") + if err != nil { + t.Fatal("could not read downloaded file") + } + t.Log("read file content successfully") + + fileContent := string(file) + expectedString := "Hi from init-container-01\nHi from container\n" + assert.Equal(t, fileContent, expectedString, "file content doesn't match expected value") + + t.Log("clean up") + cmd = kubectl("delete", "namespace", "vk-test", "--ignore-not-found") + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatal(string(out)) + } +} + func TestPodWithCSIDriver(t *testing.T) { // delete the pod first cmd := kubectl("delete", "namespace", "vk-test", "--ignore-not-found") diff --git a/pkg/provider/aci.go b/pkg/provider/aci.go index 5f0e0dbe..14065388 100644 --- a/pkg/provider/aci.go +++ b/pkg/provider/aci.go @@ -827,11 +827,12 @@ func (p *ACIProvider) getCommand(container *v1.Container) *[]string { //get VolumeMounts declared on Container as []aci.VolumeMount func (p *ACIProvider) getVolumeMounts(container *v1.Container) *[]azaci.VolumeMount { volumeMounts := make([]azaci.VolumeMount, 0, len(container.VolumeMounts)) - for _, v := range container.VolumeMounts { + fmt.Println(container.VolumeMounts) + for i := range container.VolumeMounts { volumeMounts = append(volumeMounts, azaci.VolumeMount{ - Name: &v.Name, - MountPath: &v.MountPath, - ReadOnly: &v.ReadOnly, + Name: &container.VolumeMounts[i].Name, + MountPath: &container.VolumeMounts[i].MountPath, + ReadOnly: &container.VolumeMounts[i].ReadOnly, }) } return &volumeMounts @@ -884,9 +885,9 @@ func (p *ACIProvider) getInitContainers(ctx context.Context, pod *v1.Pod) ([]aza Name: &pod.Spec.InitContainers[i].Name, InitContainerPropertiesDefinition: &azaci.InitContainerPropertiesDefinition { Image: &pod.Spec.InitContainers[i].Image, - Command: p.getCommand(&initContainer), - VolumeMounts: p.getVolumeMounts(&initContainer), - EnvironmentVariables: p.getEnvironmentVariables(&initContainer), + Command: p.getCommand(&pod.Spec.InitContainers[i]), + VolumeMounts: p.getVolumeMounts(&pod.Spec.InitContainers[i]), + EnvironmentVariables: p.getEnvironmentVariables(&pod.Spec.InitContainers[i]), }, } From d69adb94de42e863ff15a3e3740ef04cd1f380e1 Mon Sep 17 00:00:00 2001 From: Arnav Arnav Date: Thu, 17 Nov 2022 13:50:33 -0800 Subject: [PATCH 43/46] moved initcontainer e2e tests to a separate file --- e2e/initcontainer_test.go | 176 ++++++++++++++++++++++++++++++++++++++ e2e/pods_test.go | 168 ------------------------------------ 2 files changed, 176 insertions(+), 168 deletions(-) create mode 100644 e2e/initcontainer_test.go diff --git a/e2e/initcontainer_test.go b/e2e/initcontainer_test.go new file mode 100644 index 00000000..02a18a7d --- /dev/null +++ b/e2e/initcontainer_test.go @@ -0,0 +1,176 @@ + +package e2e + +import ( + "testing" + "time" + "io/ioutil" + "os/exec" + "os" + + "gotest.tools/assert" +) + +func TestPodWithInitContainers(t *testing.T) { + // delete the namespace first + cmd := kubectl("delete", "namespace", "vk-test", "--ignore-not-found") + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatal(string(out)) + } + + // create namespace + cmd = kubectl("apply", "-f", "fixtures/namespace.yml") + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatal(string(out)) + } + + testStorageAccount := os.Getenv("CSI_DRIVER_STORAGE_ACCOUNT_NAME") + testStorageKey := os.Getenv("CSI_DRIVER_STORAGE_ACCOUNT_KEY") + + cmd = kubectl("create", "secret", "generic", "csidriversecret", "--from-literal", "azurestorageaccountname="+testStorageAccount, "--from-literal", "azurestorageaccountkey="+testStorageKey, "--namespace=vk-test") + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatal(string(out)) + } + + cmd = kubectl("apply", "-f", "fixtures/initcontainers_pod.yml") + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatal(string(out)) + } + deadline, ok := t.Deadline() + timeout := time.Until(deadline) + if !ok { + timeout = 300 * time.Second + } + cmd = kubectl("wait", "--for=condition=ready", "--timeout="+timeout.String(), "pod/vk-e2e-initcontainers", "--namespace=vk-test") + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatal(string(out)) + } + t.Log("success create pod") + + // query metrics + deadline = time.Now().Add(10 * time.Minute) + for { + t.Log("query metrics ....") + cmd = kubectl("get", "--raw", "/apis/metrics.k8s.io/v1beta1/namespaces/vk-test/pods/vk-e2e-initcontainers") + out, err := cmd.CombinedOutput() + if time.Now().After(deadline) { + t.Fatal("failed to query pod's stats from metrics server API") + } + if err == nil { + t.Logf("success query metrics %s", string(out)) + break + } + time.Sleep(10 * time.Second) + } + + // check pod status + t.Log("get pod status ....") + cmd = kubectl("get", "pod", "--field-selector=status.phase=Running", "--namespace=vk-test", "--output=jsonpath={.items..metadata.name}") + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatal(string(out)) + } + if string(out) != "vk-e2e-initcontainers" { + t.Fatal("failed to get pod's status") + } + t.Logf("success query pod status %s", string(out)) + + // check container status + t.Log("get container status ....") + cmd = kubectl("get", "pod", "vk-e2e-initcontainers", "--namespace=vk-test", "--output=jsonpath={.status.containerStatuses[0].ready}") + out, err = cmd.CombinedOutput() + if err != nil { + t.Fatal(string(out)) + } + if string(out) != "true" { + t.Fatal("failed to get pod's status") + } + t.Logf("success query container status %s", string(out)) + + t.Log("clean up") + cmd = kubectl("delete", "namespace", "vk-test", "--ignore-not-found") + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatal(string(out)) + } +} + +func TestPodWithInitContainersOrder(t *testing.T) { + // delete the namespace first + cmd := kubectl("delete", "namespace", "vk-test", "--ignore-not-found") + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatal(string(out)) + } + + // create namespace + cmd = kubectl("apply", "-f", "fixtures/namespace.yml") + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatal(string(out)) + } + + testStorageAccount := os.Getenv("CSI_DRIVER_STORAGE_ACCOUNT_NAME") + testStorageKey := os.Getenv("CSI_DRIVER_STORAGE_ACCOUNT_KEY") + + cmd = kubectl("create", "secret", "generic", "csidriversecret", "--from-literal", "azurestorageaccountname="+testStorageAccount, "--from-literal", "azurestorageaccountkey="+testStorageKey, "--namespace=vk-test") + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatal(string(out)) + } + + cmd = kubectl("apply", "-f", "fixtures/initcontainers_ordertest_pod.yml") + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatal(string(out)) + } + deadline, ok := t.Deadline() + timeout := time.Until(deadline) + if !ok { + timeout = 300 * time.Second + } + cmd = kubectl("wait", "--for=condition=ready", "--timeout="+timeout.String(), "pod/vk-e2e-initcontainers-order", "--namespace=vk-test") + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatal(string(out)) + } + t.Log("success create pod") + + // download file created by pod + cmd = exec.Command("az", "storage", "file", "download", "--account-name", testStorageAccount, "--account-key", testStorageKey, "-s", "vncsidriversharename", "-p", "newfile.txt") + cmd.Env = os.Environ() + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatal(string(out)) + } + t.Log("file newfile.txt downloaded from storage account") + + file, err := ioutil.ReadFile("newfile.txt") + if err != nil { + t.Fatal("could not read downloaded file") + } + t.Log("read file content successfully") + + fileContent := string(file) + expectedString := "Hi from init-container-01\nHi from container\n" + assert.Equal(t, fileContent, expectedString, "file content doesn't match expected value") + + // check pod status + t.Log("get pod status ....") + cmd = kubectl("get", "pod", "--field-selector=status.phase=Running", "--namespace=vk-test", "--output=jsonpath={.items..metadata.name}") + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatal(string(out)) + } + if string(out) != "vk-e2e-initcontainers-order" { + t.Fatal("failed to get pod's status") + } + t.Logf("success query pod status %s", string(out)) + + // check container status + t.Log("get container status ....") + cmd = kubectl("get", "pod", "vk-e2e-initcontainers-order", "--namespace=vk-test", "--output=jsonpath={.status.containerStatuses[0].ready}") + if string(out) != "true" { + t.Fatal("failed to get pod's status") + } + t.Logf("success query container status %s", string(out)) + + t.Log("clean up pod") + cmd = kubectl("delete", "namespace", "vk-test", "--ignore-not-found") + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatal(string(out)) + } +} diff --git a/e2e/pods_test.go b/e2e/pods_test.go index e480e0a8..7baac2f7 100644 --- a/e2e/pods_test.go +++ b/e2e/pods_test.go @@ -4,11 +4,6 @@ import ( "strings" "testing" "time" - "io/ioutil" - "os/exec" - "os" - - "gotest.tools/assert" ) func TestPodLifecycle(t *testing.T) { @@ -108,166 +103,3 @@ func TestPodLifecycle(t *testing.T) { } } -func TestPodWithInitContainers(t *testing.T) { - // delete the namespace first - cmd := kubectl("delete", "namespace", "vk-test", "--ignore-not-found") - if out, err := cmd.CombinedOutput(); err != nil { - t.Fatal(string(out)) - } - - // create namespace - cmd = kubectl("apply", "-f", "fixtures/namespace.yml") - if out, err := cmd.CombinedOutput(); err != nil { - t.Fatal(string(out)) - } - - testStorageAccount := os.Getenv("CSI_DRIVER_STORAGE_ACCOUNT_NAME") - testStorageKey := os.Getenv("CSI_DRIVER_STORAGE_ACCOUNT_KEY") - - cmd = kubectl("create", "secret", "generic", "csidriversecret", "--from-literal", "azurestorageaccountname="+testStorageAccount, "--from-literal", "azurestorageaccountkey="+testStorageKey, "--namespace=vk-test") - if out, err := cmd.CombinedOutput(); err != nil { - t.Fatal(string(out)) - } - - cmd = kubectl("apply", "-f", "fixtures/initcontainers_pod.yml") - if out, err := cmd.CombinedOutput(); err != nil { - t.Fatal(string(out)) - } - deadline, ok := t.Deadline() - timeout := time.Until(deadline) - if !ok { - timeout = 300 * time.Second - } - cmd = kubectl("wait", "--for=condition=ready", "--timeout="+timeout.String(), "pod/vk-e2e-initcontainers", "--namespace=vk-test") - if out, err := cmd.CombinedOutput(); err != nil { - t.Fatal(string(out)) - } - t.Log("success create pod") - - // query metrics - deadline = time.Now().Add(10 * time.Minute) - for { - t.Log("query metrics ....") - cmd = kubectl("get", "--raw", "/apis/metrics.k8s.io/v1beta1/namespaces/vk-test/pods/vk-e2e-initcontainers") - out, err := cmd.CombinedOutput() - if time.Now().After(deadline) { - t.Fatal("failed to query pod's stats from metrics server API") - } - if err == nil { - t.Logf("success query metrics %s", string(out)) - break - } - time.Sleep(10 * time.Second) - } - - // check pod status - t.Log("get pod status ....") - cmd = kubectl("get", "pod", "--field-selector=status.phase=Running", "--namespace=vk-test", "--output=jsonpath={.items..metadata.name}") - out, err := cmd.CombinedOutput() - if err != nil { - t.Fatal(string(out)) - } - if string(out) != "vk-e2e-initcontainers" { - t.Fatal("failed to get pod's status") - } - t.Logf("success query pod status %s", string(out)) - - // check container status - t.Log("get container status ....") - cmd = kubectl("get", "pod", "vk-e2e-initcontainers", "--namespace=vk-test", "--output=jsonpath={.status.containerStatuses[0].ready}") - out, err = cmd.CombinedOutput() - if err != nil { - t.Fatal(string(out)) - } - if string(out) != "true" { - t.Fatal("failed to get pod's status") - } - t.Logf("success query container status %s", string(out)) - - t.Log("clean up") - cmd = kubectl("delete", "namespace", "vk-test", "--ignore-not-found") - if out, err := cmd.CombinedOutput(); err != nil { - t.Fatal(string(out)) - } -} - -func TestPodWithInitContainersOrder(t *testing.T) { - // delete the namespace first - cmd := kubectl("delete", "namespace", "vk-test", "--ignore-not-found") - if out, err := cmd.CombinedOutput(); err != nil { - t.Fatal(string(out)) - } - - // create namespace - cmd = kubectl("apply", "-f", "fixtures/namespace.yml") - if out, err := cmd.CombinedOutput(); err != nil { - t.Fatal(string(out)) - } - - testStorageAccount := os.Getenv("CSI_DRIVER_STORAGE_ACCOUNT_NAME") - testStorageKey := os.Getenv("CSI_DRIVER_STORAGE_ACCOUNT_KEY") - - cmd = kubectl("create", "secret", "generic", "csidriversecret", "--from-literal", "azurestorageaccountname="+testStorageAccount, "--from-literal", "azurestorageaccountkey="+testStorageKey, "--namespace=vk-test") - if out, err := cmd.CombinedOutput(); err != nil { - t.Fatal(string(out)) - } - - cmd = kubectl("apply", "-f", "fixtures/initcontainers_ordertest_pod.yml") - if out, err := cmd.CombinedOutput(); err != nil { - t.Fatal(string(out)) - } - deadline, ok := t.Deadline() - timeout := time.Until(deadline) - if !ok { - timeout = 300 * time.Second - } - cmd = kubectl("wait", "--for=condition=ready", "--timeout="+timeout.String(), "pod/vk-e2e-initcontainers-order", "--namespace=vk-test") - if out, err := cmd.CombinedOutput(); err != nil { - t.Fatal(string(out)) - } - t.Log("success create pod") - - // download file created by pod - cmd = exec.Command("az", "storage", "file", "download", "--account-name", testStorageAccount, "--account-key", testStorageKey, "-s", "vncsidriversharename", "-p", "newfile.txt") - cmd.Env = os.Environ() - if out, err := cmd.CombinedOutput(); err != nil { - t.Fatal(string(out)) - } - t.Log("file newfile.txt downloaded from storage account") - - file, err := ioutil.ReadFile("newfile.txt") - if err != nil { - t.Fatal("could not read downloaded file") - } - t.Log("read file content successfully") - - fileContent := string(file) - expectedString := "Hi from init-container-01\nHi from container\n" - assert.Equal(t, fileContent, expectedString, "file content doesn't match expected value") - - // check pod status - t.Log("get pod status ....") - cmd = kubectl("get", "pod", "--field-selector=status.phase=Running", "--namespace=vk-test", "--output=jsonpath={.items..metadata.name}") - out, err := cmd.CombinedOutput() - if err != nil { - t.Fatal(string(out)) - } - if string(out) != "vk-e2e-initcontainers-order" { - t.Fatal("failed to get pod's status") - } - t.Logf("success query pod status %s", string(out)) - - // check container status - t.Log("get container status ....") - cmd = kubectl("get", "pod", "vk-e2e-initcontainers-order", "--namespace=vk-test", "--output=jsonpath={.status.containerStatuses[0].ready}") - if string(out) != "true" { - t.Fatal("failed to get pod's status") - } - t.Logf("success query container status %s", string(out)) - - t.Log("clean up pod") - cmd = kubectl("delete", "namespace", "vk-test", "--ignore-not-found") - if out, err := cmd.CombinedOutput(); err != nil { - t.Fatal(string(out)) - } -} From 096f9b4c40bf70cfc453016e2f4ced0eecc4803d Mon Sep 17 00:00:00 2001 From: Arnav Arnav Date: Thu, 17 Nov 2022 16:31:19 -0800 Subject: [PATCH 44/46] fix merge error --- e2e/pods_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/e2e/pods_test.go b/e2e/pods_test.go index 7baac2f7..0797f23a 100644 --- a/e2e/pods_test.go +++ b/e2e/pods_test.go @@ -78,6 +78,10 @@ func TestPodLifecycle(t *testing.T) { // check container logs t.Log("get container logs ....") cmd = kubectl("logs", "pod/vk-e2e-hpa", "-c", "hpa-example", "--namespace=vk-test") + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatal(string(out)) + } if string(out) == "" { t.Fatal("failed to get container's logs") } From e6ca1f8ff966234c03921782e2afbda31e98c226 Mon Sep 17 00:00:00 2001 From: Arnav Arnav Date: Thu, 17 Nov 2022 17:24:33 -0800 Subject: [PATCH 45/46] fix lint error --- e2e/pods_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e/pods_test.go b/e2e/pods_test.go index 0797f23a..94f03e6a 100644 --- a/e2e/pods_test.go +++ b/e2e/pods_test.go @@ -78,7 +78,7 @@ func TestPodLifecycle(t *testing.T) { // check container logs t.Log("get container logs ....") cmd = kubectl("logs", "pod/vk-e2e-hpa", "-c", "hpa-example", "--namespace=vk-test") - out, err := cmd.CombinedOutput() + out, err = cmd.CombinedOutput() if err != nil { t.Fatal(string(out)) } From 60e6aebed6ec9b2754a445488defa777ccfbbc5c Mon Sep 17 00:00:00 2001 From: Arnav Arnav Date: Fri, 18 Nov 2022 12:18:27 -0800 Subject: [PATCH 46/46] combined initcontainer test; remove println; use index --- e2e/fixtures/initcontainers_ordertest_pod.yml | 7 ++ e2e/fixtures/initcontainers_pod.yml | 45 ----------- e2e/initcontainer_test.go | 81 +++---------------- pkg/provider/aci.go | 7 +- 4 files changed, 19 insertions(+), 121 deletions(-) delete mode 100644 e2e/fixtures/initcontainers_pod.yml diff --git a/e2e/fixtures/initcontainers_ordertest_pod.yml b/e2e/fixtures/initcontainers_ordertest_pod.yml index 11ed8c27..1f3c554c 100644 --- a/e2e/fixtures/initcontainers_ordertest_pod.yml +++ b/e2e/fixtures/initcontainers_ordertest_pod.yml @@ -12,6 +12,13 @@ spec: volumeMounts: - name: azure mountPath: /mnt/azure + - image: alpine + name: init-container-02 + command: [ "/bin/sh" ] + args: [ "-c", "echo Hi from init-container-02 >> /mnt/azure/newfile.txt" ] + volumeMounts: + - name: azure + mountPath: /mnt/azure containers: - image: alpine imagePullPolicy: Always diff --git a/e2e/fixtures/initcontainers_pod.yml b/e2e/fixtures/initcontainers_pod.yml deleted file mode 100644 index c136ecaa..00000000 --- a/e2e/fixtures/initcontainers_pod.yml +++ /dev/null @@ -1,45 +0,0 @@ -apiVersion: v1 -kind: Pod -metadata: - name: vk-e2e-initcontainers - namespace: vk-test -spec: - initContainers: - - image: alpine - name: init-container-01 - command: [ "/bin/sh" ] - args: [ "-c", "echo \"Hi\"" ] - - image: alpine - name: init-container-02 - command: [ "/bin/sh" ] - args: [ "-c", "echo \"Hi\"" ] - containers: - - image: alpine - imagePullPolicy: Always - name: container - command: [ - "sh", - "-c", - "while sleep 10; do echo pod with init container; done;" - ] - resources: - requests: - memory: 1G - cpu: 1 - volumeMounts: - - name: azure - mountPath: /mnt/azure - nodeSelector: - kubernetes.io/role: agent - beta.kubernetes.io/os: linux - type: virtual-kubelet - tolerations: - - key: virtual-kubelet.io/provider - operator: Exists - volumes: - - name: azure - csi: - driver: file.csi.azure.com - volumeAttributes: - secretName: csidriversecret # required - shareName: vncsidriversharename # required diff --git a/e2e/initcontainer_test.go b/e2e/initcontainer_test.go index 02a18a7d..f9651705 100644 --- a/e2e/initcontainer_test.go +++ b/e2e/initcontainer_test.go @@ -11,7 +11,7 @@ import ( "gotest.tools/assert" ) -func TestPodWithInitContainers(t *testing.T) { +func TestPodWithInitContainersOrder(t *testing.T) { // delete the namespace first cmd := kubectl("delete", "namespace", "vk-test", "--ignore-not-found") if out, err := cmd.CombinedOutput(); err != nil { @@ -32,7 +32,7 @@ func TestPodWithInitContainers(t *testing.T) { t.Fatal(string(out)) } - cmd = kubectl("apply", "-f", "fixtures/initcontainers_pod.yml") + cmd = kubectl("apply", "-f", "fixtures/initcontainers_ordertest_pod.yml") if out, err := cmd.CombinedOutput(); err != nil { t.Fatal(string(out)) } @@ -41,7 +41,7 @@ func TestPodWithInitContainers(t *testing.T) { if !ok { timeout = 300 * time.Second } - cmd = kubectl("wait", "--for=condition=ready", "--timeout="+timeout.String(), "pod/vk-e2e-initcontainers", "--namespace=vk-test") + cmd = kubectl("wait", "--for=condition=ready", "--timeout="+timeout.String(), "pod/vk-e2e-initcontainers-order", "--namespace=vk-test") if out, err := cmd.CombinedOutput(); err != nil { t.Fatal(string(out)) } @@ -51,7 +51,7 @@ func TestPodWithInitContainers(t *testing.T) { deadline = time.Now().Add(10 * time.Minute) for { t.Log("query metrics ....") - cmd = kubectl("get", "--raw", "/apis/metrics.k8s.io/v1beta1/namespaces/vk-test/pods/vk-e2e-initcontainers") + cmd = kubectl("get", "--raw", "/apis/metrics.k8s.io/v1beta1/namespaces/vk-test/pods/vk-e2e-initcontainers-order") out, err := cmd.CombinedOutput() if time.Now().After(deadline) { t.Fatal("failed to query pod's stats from metrics server API") @@ -63,73 +63,6 @@ func TestPodWithInitContainers(t *testing.T) { time.Sleep(10 * time.Second) } - // check pod status - t.Log("get pod status ....") - cmd = kubectl("get", "pod", "--field-selector=status.phase=Running", "--namespace=vk-test", "--output=jsonpath={.items..metadata.name}") - out, err := cmd.CombinedOutput() - if err != nil { - t.Fatal(string(out)) - } - if string(out) != "vk-e2e-initcontainers" { - t.Fatal("failed to get pod's status") - } - t.Logf("success query pod status %s", string(out)) - - // check container status - t.Log("get container status ....") - cmd = kubectl("get", "pod", "vk-e2e-initcontainers", "--namespace=vk-test", "--output=jsonpath={.status.containerStatuses[0].ready}") - out, err = cmd.CombinedOutput() - if err != nil { - t.Fatal(string(out)) - } - if string(out) != "true" { - t.Fatal("failed to get pod's status") - } - t.Logf("success query container status %s", string(out)) - - t.Log("clean up") - cmd = kubectl("delete", "namespace", "vk-test", "--ignore-not-found") - if out, err := cmd.CombinedOutput(); err != nil { - t.Fatal(string(out)) - } -} - -func TestPodWithInitContainersOrder(t *testing.T) { - // delete the namespace first - cmd := kubectl("delete", "namespace", "vk-test", "--ignore-not-found") - if out, err := cmd.CombinedOutput(); err != nil { - t.Fatal(string(out)) - } - - // create namespace - cmd = kubectl("apply", "-f", "fixtures/namespace.yml") - if out, err := cmd.CombinedOutput(); err != nil { - t.Fatal(string(out)) - } - - testStorageAccount := os.Getenv("CSI_DRIVER_STORAGE_ACCOUNT_NAME") - testStorageKey := os.Getenv("CSI_DRIVER_STORAGE_ACCOUNT_KEY") - - cmd = kubectl("create", "secret", "generic", "csidriversecret", "--from-literal", "azurestorageaccountname="+testStorageAccount, "--from-literal", "azurestorageaccountkey="+testStorageKey, "--namespace=vk-test") - if out, err := cmd.CombinedOutput(); err != nil { - t.Fatal(string(out)) - } - - cmd = kubectl("apply", "-f", "fixtures/initcontainers_ordertest_pod.yml") - if out, err := cmd.CombinedOutput(); err != nil { - t.Fatal(string(out)) - } - deadline, ok := t.Deadline() - timeout := time.Until(deadline) - if !ok { - timeout = 300 * time.Second - } - cmd = kubectl("wait", "--for=condition=ready", "--timeout="+timeout.String(), "pod/vk-e2e-initcontainers-order", "--namespace=vk-test") - if out, err := cmd.CombinedOutput(); err != nil { - t.Fatal(string(out)) - } - t.Log("success create pod") - // download file created by pod cmd = exec.Command("az", "storage", "file", "download", "--account-name", testStorageAccount, "--account-key", testStorageKey, "-s", "vncsidriversharename", "-p", "newfile.txt") cmd.Env = os.Environ() @@ -145,7 +78,7 @@ func TestPodWithInitContainersOrder(t *testing.T) { t.Log("read file content successfully") fileContent := string(file) - expectedString := "Hi from init-container-01\nHi from container\n" + expectedString := "Hi from init-container-01\nHi from init-container-02\nHi from container\n" assert.Equal(t, fileContent, expectedString, "file content doesn't match expected value") // check pod status @@ -163,6 +96,10 @@ func TestPodWithInitContainersOrder(t *testing.T) { // check container status t.Log("get container status ....") cmd = kubectl("get", "pod", "vk-e2e-initcontainers-order", "--namespace=vk-test", "--output=jsonpath={.status.containerStatuses[0].ready}") + out, err = cmd.CombinedOutput() + if err != nil { + t.Fatal(string(out)) + } if string(out) != "true" { t.Fatal("failed to get pod's status") } diff --git a/pkg/provider/aci.go b/pkg/provider/aci.go index 1ba3ea3e..17d1e4de 100644 --- a/pkg/provider/aci.go +++ b/pkg/provider/aci.go @@ -865,7 +865,6 @@ func (p *ACIProvider) getCommand(container *v1.Container) *[]string { //get VolumeMounts declared on Container as []aci.VolumeMount func (p *ACIProvider) getVolumeMounts(container *v1.Container) *[]azaci.VolumeMount { volumeMounts := make([]azaci.VolumeMount, 0, len(container.VolumeMounts)) - fmt.Println(container.VolumeMounts) for i := range container.VolumeMounts { volumeMounts = append(volumeMounts, azaci.VolumeMount{ Name: &container.VolumeMounts[i].Name, @@ -879,9 +878,9 @@ func (p *ACIProvider) getVolumeMounts(container *v1.Container) *[]azaci.VolumeMo //get EnvironmentVariables declared on Container as []aci.EnvironmentVariable func (p *ACIProvider) getEnvironmentVariables(container *v1.Container) *[]azaci.EnvironmentVariable { environmentVariable := make([]azaci.EnvironmentVariable, 0, len(container.Env)) - for _, e := range container.Env { - if e.Value != "" { - envVar := getACIEnvVar(e) + for i := range container.Env { + if container.Env[i].Value != "" { + envVar := getACIEnvVar(container.Env[i]) environmentVariable = append(environmentVariable, envVar) } }