Skip to content
This repository was archived by the owner on Oct 22, 2024. It is now read-only.

Commit 2a9c10f

Browse files
committed
operator: fix conversion web hook
Not all fields were getting copied. The test was incomplete and therefore didn't catch that mistake.
1 parent 053e442 commit 2a9c10f

File tree

2 files changed

+74
-27
lines changed

2 files changed

+74
-27
lines changed

pkg/apis/pmemcsi/v1alpha1/deployment_conversion.go

+16-4
Original file line numberDiff line numberDiff line change
@@ -37,15 +37,21 @@ func (d *Deployment) ConvertTo(dst conversion.Hub) error {
3737

3838
// no change in other fields
3939
out.ObjectMeta = in.ObjectMeta
40-
out.Spec.LogLevel = in.Spec.LogLevel
4140
out.Spec.Image = in.Spec.Image
42-
out.Spec.CACert = in.Spec.CACert
41+
out.Spec.PullPolicy = in.Spec.PullPolicy
42+
out.Spec.ProvisionerImage = in.Spec.ProvisionerImage
43+
out.Spec.NodeRegistrarImage = in.Spec.NodeRegistrarImage
44+
out.Spec.DeviceMode = v1beta1.DeviceMode(string(in.Spec.DeviceMode))
45+
out.Spec.LogLevel = in.Spec.LogLevel
4346
out.Spec.RegistryCert = in.Spec.RegistryCert
4447
out.Spec.RegistryPrivateKey = in.Spec.RegistryPrivateKey
4548
out.Spec.NodeControllerCert = in.Spec.NodeControllerCert
4649
out.Spec.NodeControllerPrivateKey = in.Spec.NodeControllerPrivateKey
50+
out.Spec.CACert = in.Spec.CACert
4751
out.Spec.NodeSelector = in.Spec.NodeSelector
52+
out.Spec.PMEMPercentage = in.Spec.PMEMPercentage
4853
out.Spec.Labels = in.Spec.Labels
54+
out.Spec.KubeletDir = in.Spec.KubeletDir
4955

5056
out.Status.Components = nil
5157
for _, s := range in.Status.Components {
@@ -88,15 +94,21 @@ func (d *Deployment) ConvertFrom(src conversion.Hub) error {
8894

8995
// no change in other fields
9096
out.ObjectMeta = in.ObjectMeta
91-
out.Spec.LogLevel = in.Spec.LogLevel
9297
out.Spec.Image = in.Spec.Image
93-
out.Spec.CACert = in.Spec.CACert
98+
out.Spec.PullPolicy = in.Spec.PullPolicy
99+
out.Spec.ProvisionerImage = in.Spec.ProvisionerImage
100+
out.Spec.NodeRegistrarImage = in.Spec.NodeRegistrarImage
101+
out.Spec.DeviceMode = DeviceMode(string(in.Spec.DeviceMode))
102+
out.Spec.LogLevel = in.Spec.LogLevel
94103
out.Spec.RegistryCert = in.Spec.RegistryCert
95104
out.Spec.RegistryPrivateKey = in.Spec.RegistryPrivateKey
96105
out.Spec.NodeControllerCert = in.Spec.NodeControllerCert
97106
out.Spec.NodeControllerPrivateKey = in.Spec.NodeControllerPrivateKey
107+
out.Spec.CACert = in.Spec.CACert
98108
out.Spec.NodeSelector = in.Spec.NodeSelector
109+
out.Spec.PMEMPercentage = in.Spec.PMEMPercentage
99110
out.Spec.Labels = in.Spec.Labels
111+
out.Spec.KubeletDir = in.Spec.KubeletDir
100112

101113
out.Status.Components = nil
102114
for _, s := range in.Status.Components {

test/e2e/operator/deployment_api.go

+58-23
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"context"
1111
"fmt"
1212
"os"
13+
"reflect"
1314
"strings"
1415
"time"
1516

@@ -947,25 +948,41 @@ var _ = deploy.DescribeForSome("API", func(d *deploy.Deployment) bool {
947948
Expect(alphaCR.Status).Should(BeEquivalentTo(cr.Status), "status mismatch")
948949
})
949950
It("with explicit values", func() {
950-
alphaDep := getAlphaDeployment("alpha-explicit-values")
951-
alphaDep.Spec.NodeResources = &corev1.ResourceRequirements{
952-
Requests: corev1.ResourceList{
953-
corev1.ResourceCPU: resource.MustParse("10m"),
954-
corev1.ResourceMemory: resource.MustParse("25Mi"),
955-
},
956-
Limits: corev1.ResourceList{
957-
corev1.ResourceCPU: resource.MustParse("100m"),
958-
corev1.ResourceMemory: resource.MustParse("50Mi"),
959-
},
960-
}
961-
alphaDep.Spec.ControllerResources = &corev1.ResourceRequirements{
962-
Requests: corev1.ResourceList{
963-
corev1.ResourceCPU: resource.MustParse("20m"),
964-
corev1.ResourceMemory: resource.MustParse("50Mi"),
951+
alphaDep := alphaapi.Deployment{
952+
ObjectMeta: metav1.ObjectMeta{
953+
Name: "explict-values",
965954
},
966-
Limits: corev1.ResourceList{
967-
corev1.ResourceCPU: resource.MustParse("200m"),
968-
corev1.ResourceMemory: resource.MustParse("100Mi"),
955+
Spec: alphaapi.DeploymentSpec{
956+
Image: dummyImage,
957+
PullPolicy: corev1.PullAlways,
958+
ProvisionerImage: "no-such-provisioner",
959+
NodeRegistrarImage: "no-such-registrar",
960+
NodeResources: &corev1.ResourceRequirements{
961+
Requests: corev1.ResourceList{
962+
corev1.ResourceCPU: resource.MustParse("10m"),
963+
corev1.ResourceMemory: resource.MustParse("25Mi"),
964+
},
965+
Limits: corev1.ResourceList{
966+
corev1.ResourceCPU: resource.MustParse("100m"),
967+
corev1.ResourceMemory: resource.MustParse("50Mi"),
968+
},
969+
},
970+
ControllerResources: &corev1.ResourceRequirements{
971+
Requests: corev1.ResourceList{
972+
corev1.ResourceCPU: resource.MustParse("20m"),
973+
corev1.ResourceMemory: resource.MustParse("50Mi"),
974+
},
975+
Limits: corev1.ResourceList{
976+
corev1.ResourceCPU: resource.MustParse("200m"),
977+
corev1.ResourceMemory: resource.MustParse("100Mi"),
978+
},
979+
},
980+
DeviceMode: alphaapi.DeviceModeDirect,
981+
LogLevel: 5,
982+
NodeSelector: map[string]string{"no-such-label": "no-such-key"},
983+
PMEMPercentage: 99,
984+
Labels: map[string]string{"app": "explicit"},
985+
KubeletDir: "/tmp",
969986
},
970987
}
971988
deploy.CreateAlphaDeploymentCR(f, alphaDep)
@@ -985,14 +1002,32 @@ var _ = deploy.DescribeForSome("API", func(d *deploy.Deployment) bool {
9851002

9861003
defer deploy.DeleteDeploymentCR(f, deployment.Name)
9871004

988-
validateDriver(deployment, true)
989-
990-
// Expect to ruturn alpha object converting from stored beta CR
1005+
// Expect the same spec to be returned for the
1006+
// stored CR, regardless of the version that
1007+
// is used to retrieve it.
9911008
alphaCR := deploy.GetAlphaDeploymentCR(f, deployment.Name)
992-
betaCR := deploy.GetDeploymentCR(f, deployment.Name)
1009+
cr := deploy.GetDeploymentCR(f, deployment.Name)
9931010
Expect(alphaCR).ShouldNot(BeNil(), "get alpha CR")
9941011
Expect(alphaCR.Spec).Should(BeEquivalentTo(alphaDep.Spec), "alpha CR spec mismatch")
995-
Expect(alphaCR.Status).Should(BeEquivalentTo(betaCR.Status), "alpha CR status mismatch")
1012+
Expect(alphaCR.Status).Should(BeEquivalentTo(cr.Status), "status mismatch")
1013+
1014+
// We can also compare the full spec by iterating over all fields. Those that have no match
1015+
// must be blanked out first.
1016+
// BeEquivalentTo cannot be used here because the structs cannot be converted into each other.
1017+
alphaDep.Spec.NodeResources = nil
1018+
alphaDep.Spec.ControllerResources = nil
1019+
alphaV := reflect.ValueOf(alphaDep.Spec)
1020+
alphaType := reflect.TypeOf(alphaDep.Spec)
1021+
v := reflect.ValueOf(cr.Spec)
1022+
for i := 0; i < alphaType.NumField(); i++ {
1023+
name := alphaType.Field(i).Name
1024+
actual := v.FieldByName(name)
1025+
if actual.Kind() == reflect.Invalid {
1026+
// Zero value, ignore the field.
1027+
continue
1028+
}
1029+
Expect(actual.Interface()).Should(BeEquivalentTo(alphaV.FieldByName(name).Interface()), "current CR field %s mismatch", name)
1030+
}
9961031
})
9971032
})
9981033
})

0 commit comments

Comments
 (0)