Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

various operator fixes #830

Merged
merged 2 commits into from
Dec 17, 2020
Merged

various operator fixes #830

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
053e442
operator: various CRD fixes (defaults, validation)
pohly Dec 15, 2020
2a9c10f
operator: fix conversion web hook
pohly Dec 16, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions pkg/apis/pmemcsi/v1alpha1/deployment_conversion.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,21 @@ func (d *Deployment) ConvertTo(dst conversion.Hub) error {

// no change in other fields
out.ObjectMeta = in.ObjectMeta
out.Spec.LogLevel = in.Spec.LogLevel
out.Spec.Image = in.Spec.Image
out.Spec.CACert = in.Spec.CACert
out.Spec.PullPolicy = in.Spec.PullPolicy
Copy link
Contributor

@avalluri avalluri Dec 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pohly I googled about this missing conversions and came across about conversion-gen code generation tool, that generates the type conversion code as mentioned here. What do you think, shall we use that as a separate change or part of this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we use it, then in a separate PR.

I want to get into a stable state as quickly as possible again and using that tool may need further investigations and discussions.

out.Spec.ProvisionerImage = in.Spec.ProvisionerImage
out.Spec.NodeRegistrarImage = in.Spec.NodeRegistrarImage
out.Spec.DeviceMode = v1beta1.DeviceMode(string(in.Spec.DeviceMode))
out.Spec.LogLevel = in.Spec.LogLevel
out.Spec.RegistryCert = in.Spec.RegistryCert
out.Spec.RegistryPrivateKey = in.Spec.RegistryPrivateKey
out.Spec.NodeControllerCert = in.Spec.NodeControllerCert
out.Spec.NodeControllerPrivateKey = in.Spec.NodeControllerPrivateKey
out.Spec.CACert = in.Spec.CACert
out.Spec.NodeSelector = in.Spec.NodeSelector
out.Spec.PMEMPercentage = in.Spec.PMEMPercentage
out.Spec.Labels = in.Spec.Labels
out.Spec.KubeletDir = in.Spec.KubeletDir

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

// no change in other fields
out.ObjectMeta = in.ObjectMeta
out.Spec.LogLevel = in.Spec.LogLevel
out.Spec.Image = in.Spec.Image
out.Spec.CACert = in.Spec.CACert
out.Spec.PullPolicy = in.Spec.PullPolicy
out.Spec.ProvisionerImage = in.Spec.ProvisionerImage
out.Spec.NodeRegistrarImage = in.Spec.NodeRegistrarImage
out.Spec.DeviceMode = DeviceMode(string(in.Spec.DeviceMode))
out.Spec.LogLevel = in.Spec.LogLevel
out.Spec.RegistryCert = in.Spec.RegistryCert
out.Spec.RegistryPrivateKey = in.Spec.RegistryPrivateKey
out.Spec.NodeControllerCert = in.Spec.NodeControllerCert
out.Spec.NodeControllerPrivateKey = in.Spec.NodeControllerPrivateKey
out.Spec.CACert = in.Spec.CACert
out.Spec.NodeSelector = in.Spec.NodeSelector
out.Spec.PMEMPercentage = in.Spec.PMEMPercentage
out.Spec.Labels = in.Spec.Labels
out.Spec.KubeletDir = in.Spec.KubeletDir

out.Status.Components = nil
for _, s := range in.Status.Components {
Expand Down
81 changes: 58 additions & 23 deletions test/e2e/operator/deployment_api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"context"
"fmt"
"os"
"reflect"
"strings"
"time"

Expand Down Expand Up @@ -947,25 +948,41 @@ var _ = deploy.DescribeForSome("API", func(d *deploy.Deployment) bool {
Expect(alphaCR.Status).Should(BeEquivalentTo(cr.Status), "status mismatch")
})
It("with explicit values", func() {
alphaDep := getAlphaDeployment("alpha-explicit-values")
alphaDep.Spec.NodeResources = &corev1.ResourceRequirements{
Requests: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("10m"),
corev1.ResourceMemory: resource.MustParse("25Mi"),
},
Limits: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("100m"),
corev1.ResourceMemory: resource.MustParse("50Mi"),
},
}
alphaDep.Spec.ControllerResources = &corev1.ResourceRequirements{
Requests: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("20m"),
corev1.ResourceMemory: resource.MustParse("50Mi"),
alphaDep := alphaapi.Deployment{
ObjectMeta: metav1.ObjectMeta{
Name: "explict-values",
},
Limits: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("200m"),
corev1.ResourceMemory: resource.MustParse("100Mi"),
Spec: alphaapi.DeploymentSpec{
Image: dummyImage,
PullPolicy: corev1.PullAlways,
ProvisionerImage: "no-such-provisioner",
NodeRegistrarImage: "no-such-registrar",
NodeResources: &corev1.ResourceRequirements{
Requests: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("10m"),
corev1.ResourceMemory: resource.MustParse("25Mi"),
},
Limits: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("100m"),
corev1.ResourceMemory: resource.MustParse("50Mi"),
},
},
ControllerResources: &corev1.ResourceRequirements{
Requests: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("20m"),
corev1.ResourceMemory: resource.MustParse("50Mi"),
},
Limits: corev1.ResourceList{
corev1.ResourceCPU: resource.MustParse("200m"),
corev1.ResourceMemory: resource.MustParse("100Mi"),
},
},
DeviceMode: alphaapi.DeviceModeDirect,
LogLevel: 5,
NodeSelector: map[string]string{"no-such-label": "no-such-key"},
PMEMPercentage: 99,
Labels: map[string]string{"app": "explicit"},
KubeletDir: "/tmp",
},
}
deploy.CreateAlphaDeploymentCR(f, alphaDep)
Expand All @@ -985,14 +1002,32 @@ var _ = deploy.DescribeForSome("API", func(d *deploy.Deployment) bool {

defer deploy.DeleteDeploymentCR(f, deployment.Name)

validateDriver(deployment, true)

// Expect to ruturn alpha object converting from stored beta CR
// Expect the same spec to be returned for the
// stored CR, regardless of the version that
// is used to retrieve it.
alphaCR := deploy.GetAlphaDeploymentCR(f, deployment.Name)
betaCR := deploy.GetDeploymentCR(f, deployment.Name)
cr := deploy.GetDeploymentCR(f, deployment.Name)
Expect(alphaCR).ShouldNot(BeNil(), "get alpha CR")
Expect(alphaCR.Spec).Should(BeEquivalentTo(alphaDep.Spec), "alpha CR spec mismatch")
Expect(alphaCR.Status).Should(BeEquivalentTo(betaCR.Status), "alpha CR status mismatch")
Expect(alphaCR.Status).Should(BeEquivalentTo(cr.Status), "status mismatch")

// We can also compare the full spec by iterating over all fields. Those that have no match
// must be blanked out first.
// BeEquivalentTo cannot be used here because the structs cannot be converted into each other.
alphaDep.Spec.NodeResources = nil
alphaDep.Spec.ControllerResources = nil
alphaV := reflect.ValueOf(alphaDep.Spec)
alphaType := reflect.TypeOf(alphaDep.Spec)
v := reflect.ValueOf(cr.Spec)
for i := 0; i < alphaType.NumField(); i++ {
name := alphaType.Field(i).Name
actual := v.FieldByName(name)
if actual.Kind() == reflect.Invalid {
// Zero value, ignore the field.
continue
}
Expect(actual.Interface()).Should(BeEquivalentTo(alphaV.FieldByName(name).Interface()), "current CR field %s mismatch", name)
}
})
})
})
Expand Down