Skip to content

Commit

Permalink
Provide virtio-win pullspec in dedicated ConfigMap (#1647)
Browse files Browse the repository at this point in the history
* [Bug Fix 1984954]: Provide virtio-win pullspec in dedicated ConfigMap

Signed-off-by: orenc1 <ocohen@redhat.com>

* Fix unit tests mocking also new resources

Signed-off-by: Simone Tiraboschi <stirabos@redhat.com>

* Generalize handlers for ConfigMaps, Roles and RoleBindings

Signed-off-by: orenc1 <ocohen@redhat.com>

Co-authored-by: Simone Tiraboschi <stirabos@redhat.com>
  • Loading branch information
orenc1 and tiraboschi committed Dec 13, 2021
1 parent 013aff0 commit f06ce4b
Show file tree
Hide file tree
Showing 15 changed files with 628 additions and 196 deletions.
7 changes: 7 additions & 0 deletions hack/upgrade-test-index-image.sh
Expand Up @@ -311,6 +311,13 @@ fi
Msg "check golden images"
KUBECTL_BINARY=${CMD} INSTALLED_NAMESPACE=${HCO_NAMESPACE} ./hack/check_defaults.sh

Msg "check virtio-win image is in configmap"
VIRTIOWIN_IMAGE_CSV=$(${CMD} get ${CSV} -n ${HCO_NAMESPACE} \
-o jsonpath='{.spec.install.spec.deployments[?(@.name=="hco-operator")].spec.template.spec.containers[0].env[?(@.name=="VIRTIOWIN_CONTAINER")].value}')
VIRTIOWIN_IMAGE_CM=$(${CMD} get cm virtio-win -n ${HCO_NAMESPACE} -o jsonpath='{.data.virtio-win-image}')

[[ "${VIRTIOWIN_IMAGE_CSV}" == "${VIRTIOWIN_IMAGE_CM}" ]]

Msg "Read the HCO operator log before it been deleted"
HCO_POD=$( ${CMD} get -n ${HCO_NAMESPACE} pods -l "name=hyperconverged-cluster-operator" -o name)
${CMD} logs -n ${HCO_NAMESPACE} "${HCO_POD}"
Expand Down
Expand Up @@ -689,7 +689,7 @@ var _ = Describe("HyperconvergedController", func() {
).To(BeNil())

Expect(foundResource.Status.RelatedObjects).ToNot(BeNil())
Expect(len(foundResource.Status.RelatedObjects)).Should(Equal(14))
Expect(len(foundResource.Status.RelatedObjects)).Should(Equal(17))
Expect(foundResource.ObjectMeta.Finalizers).Should(Equal([]string{FinalizerName}))

// Now, delete HCO
Expand Down
20 changes: 18 additions & 2 deletions pkg/controller/hyperconverged/testUtils_test.go
Expand Up @@ -119,6 +119,9 @@ type BasicExpected struct {
cliDownload *consolev1.ConsoleCLIDownload
cliDownloadsRoute *routev1.Route
cliDownloadsService *corev1.Service
virtioWinConfig *corev1.ConfigMap
virtioWinRole *rbacv1.Role
virtioWinRoleBinding *rbacv1.RoleBinding
hcoCRD *apiextensionsv1.CustomResourceDefinition
}

Expand All @@ -139,6 +142,9 @@ func (be BasicExpected) toArray() []runtime.Object {
be.cliDownload,
be.cliDownloadsRoute,
be.cliDownloadsService,
be.virtioWinConfig,
be.virtioWinRole,
be.virtioWinRoleBinding,
be.hcoCRD,
}
}
Expand Down Expand Up @@ -184,11 +190,11 @@ func getBasicDeployment() *BasicExpected {
expectedKVStorageConfig := operands.NewKubeVirtStorageConfigForCR(hco, namespace)
expectedKVStorageConfig.ObjectMeta.SelfLink = fmt.Sprintf("/apis/v1/namespaces/%s/configmaps/%s", expectedKVStorageConfig.Namespace, expectedKVStorageConfig.Name)
res.kvStorageConfig = expectedKVStorageConfig
expectedKVStorageRole := operands.NewConfigReaderRoleForCR(hco, namespace)
expectedKVStorageRole := operands.NewCdiConfigReaderRole(hco)
expectedKVStorageRole.ObjectMeta.SelfLink = fmt.Sprintf("/apis/v1/namespaces/%s/roles/%s", expectedKVStorageConfig.Namespace, expectedKVStorageConfig.Name)
res.kvStorageRole = expectedKVStorageRole

expectedKVStorageRoleBinding := operands.NewConfigReaderRoleBindingForCR(hco, namespace)
expectedKVStorageRoleBinding := operands.NewCdiConfigReaderRoleBinding(hco)
expectedKVStorageRoleBinding.ObjectMeta.SelfLink = fmt.Sprintf("/apis/v1/namespaces/%s/rolebindings/%s", expectedKVStorageConfig.Namespace, expectedKVStorageConfig.Name)
res.kvStorageRoleBinding = expectedKVStorageRoleBinding

Expand Down Expand Up @@ -241,6 +247,16 @@ func getBasicDeployment() *BasicExpected {
expectedCliDownloadsService.SelfLink = fmt.Sprintf("/apis/v1/namespaces/%s/services/%s", expectedCliDownloadsService.Namespace, expectedCliDownloadsService.Name)
res.cliDownloadsService = expectedCliDownloadsService

expectedVirtioWinConfig, err := operands.NewVirtioWinCm(hco)
ExpectWithOffset(1, err).ToNot(HaveOccurred())
res.virtioWinConfig = expectedVirtioWinConfig

expectedVirtioWinRole := operands.NewVirtioWinCmReaderRole(hco)
res.virtioWinRole = expectedVirtioWinRole

expectedVirtioWinRoleBinding := operands.NewVirtioWinCmReaderRoleBinding(hco)
res.virtioWinRoleBinding = expectedVirtioWinRoleBinding

hcoCrd := &apiextensionsv1.CustomResourceDefinition{
ObjectMeta: metav1.ObjectMeta{
Name: "hyperconvergeds.hco.kubevirt.io",
Expand Down
114 changes: 15 additions & 99 deletions pkg/controller/operands/cdi.go
Expand Up @@ -4,6 +4,8 @@ import (
"errors"
"reflect"

log "github.com/go-logr/logr"

corev1 "k8s.io/api/core/v1"
rbacv1 "k8s.io/api/rbac/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -251,62 +253,26 @@ func NewKubeVirtStorageConfigForCR(cr *hcov1beta1.HyperConverged, namespace stri
}

// ************** Config Reader Role Handler **************
type configReaderRoleHandler genericOperand

func newConfigReaderRoleHandler(Client client.Client, Scheme *runtime.Scheme) *configReaderRoleHandler {
return &configReaderRoleHandler{
Client: Client,
Scheme: Scheme,
crType: "Role",
removeExistingOwner: false,
setControllerReference: true,
hooks: &configReaderRoleHooks{},
}
}
func NewConfigReaderRoleHandler(_ log.Logger, Client client.Client, Scheme *runtime.Scheme, hc *hcov1beta1.HyperConverged) ([]Operand, error) {
cdiConfigReaderRole := NewCdiConfigReaderRole(hc)

type configReaderRoleHooks struct{}
return []Operand{newRoleHandler(Client, Scheme, cdiConfigReaderRole)}, nil

func (h configReaderRoleHooks) getFullCr(hc *hcov1beta1.HyperConverged) (client.Object, error) {
return NewConfigReaderRoleForCR(hc, hc.Namespace), nil
}
func (h configReaderRoleHooks) getEmptyCr() client.Object { return &rbacv1.Role{} }
func (h configReaderRoleHooks) getObjectMeta(cr runtime.Object) *metav1.ObjectMeta {
return &cr.(*rbacv1.Role).ObjectMeta
}
func (h *configReaderRoleHooks) updateCr(req *common.HcoRequest, Client client.Client, exists runtime.Object, required runtime.Object) (bool, bool, error) {
configReaderRole, ok1 := required.(*rbacv1.Role)
found, ok2 := exists.(*rbacv1.Role)
if !ok1 || !ok2 {
return false, false, errors.New("can't convert to a Role")
}

if !reflect.DeepEqual(found.Labels, configReaderRole.Labels) ||
!reflect.DeepEqual(found.Rules, configReaderRole.Rules) {

req.Logger.Info("Updating existing Config Reader Role to its default values")

found.Rules = make([]rbacv1.PolicyRule, len(configReaderRole.Rules))
for i := range configReaderRole.Rules {
configReaderRole.Rules[i].DeepCopyInto(&found.Rules[i])
}
util.DeepCopyLabels(&configReaderRole.ObjectMeta, &found.ObjectMeta)

err := Client.Update(req.Ctx, found)
if err != nil {
return false, false, err
}
return true, !req.HCOTriggered, nil
}
// ************** Config Reader Role Binding Handler **************
func newConfigReaderRoleBindingHandler(_ log.Logger, Client client.Client, Scheme *runtime.Scheme, hc *hcov1beta1.HyperConverged) ([]Operand, error) {
cdiConfigReaderRoleBinding := NewCdiConfigReaderRoleBinding(hc)

return false, false, nil
return []Operand{newRoleBindingHandler(Client, Scheme, cdiConfigReaderRoleBinding)}, nil
}

func NewConfigReaderRoleForCR(cr *hcov1beta1.HyperConverged, namespace string) *rbacv1.Role {
func NewCdiConfigReaderRole(hc *hcov1beta1.HyperConverged) *rbacv1.Role {
return &rbacv1.Role{
ObjectMeta: metav1.ObjectMeta{
Name: cdiRoleName,
Labels: getLabels(cr, hcoutil.AppComponentStorage),
Namespace: namespace,
Labels: getLabels(hc, hcoutil.AppComponentStorage),
Namespace: hc.Namespace,
},
Rules: []rbacv1.PolicyRule{
{
Expand All @@ -319,62 +285,12 @@ func NewConfigReaderRoleForCR(cr *hcov1beta1.HyperConverged, namespace string) *
}
}

// ************** Config Reader Role Binding Handler **************
type configReaderRoleBindingHandler genericOperand

func newConfigReaderRoleBindingHandler(Client client.Client, Scheme *runtime.Scheme) *configReaderRoleBindingHandler {
return &configReaderRoleBindingHandler{
Client: Client,
Scheme: Scheme,
crType: "RoleBinding",
removeExistingOwner: false,
setControllerReference: true,
hooks: &configReaderRoleBindingHooks{},
}
}

type configReaderRoleBindingHooks struct{}

func (h configReaderRoleBindingHooks) getFullCr(hc *hcov1beta1.HyperConverged) (client.Object, error) {
return NewConfigReaderRoleBindingForCR(hc, hc.Namespace), nil
}
func (h configReaderRoleBindingHooks) getEmptyCr() client.Object { return &rbacv1.RoleBinding{} }
func (h configReaderRoleBindingHooks) getObjectMeta(cr runtime.Object) *metav1.ObjectMeta {
return &cr.(*rbacv1.RoleBinding).ObjectMeta
}
func (h *configReaderRoleBindingHooks) updateCr(req *common.HcoRequest, Client client.Client, exists runtime.Object, required runtime.Object) (bool, bool, error) {
configReaderRoleBinding, ok1 := required.(*rbacv1.RoleBinding)
found, ok2 := exists.(*rbacv1.RoleBinding)
if !ok1 || !ok2 {
return false, false, errors.New("can't convert to a RoleBinding")
}

if !reflect.DeepEqual(found.Labels, configReaderRoleBinding.Labels) ||
!reflect.DeepEqual(found.Subjects, configReaderRoleBinding.Subjects) ||
!reflect.DeepEqual(found.RoleRef, configReaderRoleBinding.RoleRef) {
req.Logger.Info("Updating existing Config Reader RoleBinding to its default values")

found.Subjects = make([]rbacv1.Subject, len(configReaderRoleBinding.Subjects))
copy(found.Subjects, configReaderRoleBinding.Subjects)
found.RoleRef = configReaderRoleBinding.RoleRef
util.DeepCopyLabels(&configReaderRoleBinding.ObjectMeta, &found.ObjectMeta)

err := Client.Update(req.Ctx, found)
if err != nil {
return false, false, err
}
return true, !req.HCOTriggered, nil
}

return false, false, nil
}

func NewConfigReaderRoleBindingForCR(cr *hcov1beta1.HyperConverged, namespace string) *rbacv1.RoleBinding {
func NewCdiConfigReaderRoleBinding(hc *hcov1beta1.HyperConverged) *rbacv1.RoleBinding {
return &rbacv1.RoleBinding{
ObjectMeta: metav1.ObjectMeta{
Name: cdiRoleName,
Labels: getLabels(cr, hcoutil.AppComponentStorage),
Namespace: namespace,
Labels: getLabels(hc, hcoutil.AppComponentStorage),
Namespace: hc.Namespace,
},
RoleRef: rbacv1.RoleRef{
APIGroup: "rbac.authorization.k8s.io",
Expand Down
40 changes: 24 additions & 16 deletions pkg/controller/operands/cdi_test.go
Expand Up @@ -1070,12 +1070,14 @@ var _ = Describe("CDI Operand", func() {

Context("Config Reader Role", func() {
It("should do nothing if exists", func() {
expectedRole := NewConfigReaderRoleForCR(hco, hco.Namespace)
expectedRole := NewCdiConfigReaderRole(hco)
cl := commonTestUtils.InitClient([]runtime.Object{hco, expectedRole})

handler := (*genericOperand)(newConfigReaderRoleHandler(cl, commonTestUtils.GetScheme()))
res := handler.ensure(req)
Expect(res.Err).ToNot(HaveOccurred())
handlers, err := NewConfigReaderRoleHandler(logger, cl, commonTestUtils.GetScheme(), hco)
Expect(handlers).To(HaveLen(1))
Expect(err).To(BeNil())
res := handlers[0].ensure(req)
Expect(res.Err).NotTo(HaveOccurred())

foundRole := &rbacv1.Role{}
Expect(
Expand All @@ -1089,15 +1091,17 @@ var _ = Describe("CDI Operand", func() {
})

It("should update if labels are missing", func() {
expectedRole := NewConfigReaderRoleForCR(hco, hco.Namespace)
expectedRole := NewCdiConfigReaderRole(hco)
expectedLabels := expectedRole.Labels
expectedRole.Labels = nil

cl := commonTestUtils.InitClient([]runtime.Object{hco, expectedRole})

handler := (*genericOperand)(newConfigReaderRoleHandler(cl, commonTestUtils.GetScheme()))
res := handler.ensure(req)
Expect(res.Err).ToNot(HaveOccurred())
handlers, err := NewConfigReaderRoleHandler(logger, cl, commonTestUtils.GetScheme(), hco)
Expect(handlers).To(HaveLen(1))
Expect(err).To(BeNil())
res := handlers[0].ensure(req)
Expect(res.Err).NotTo(HaveOccurred())

foundRole := &rbacv1.Role{}
Expect(
Expand All @@ -1112,13 +1116,15 @@ var _ = Describe("CDI Operand", func() {

Context("Config Reader Role Binding", func() {
It("should do nothing if exists", func() {
expectedRoleBinding := NewConfigReaderRoleBindingForCR(hco, hco.Namespace)
expectedRoleBinding := NewCdiConfigReaderRoleBinding(hco)

cl := commonTestUtils.InitClient([]runtime.Object{hco, expectedRoleBinding})

handler := (*genericOperand)(newConfigReaderRoleBindingHandler(cl, commonTestUtils.GetScheme()))
res := handler.ensure(req)
Expect(res.Err).ToNot(HaveOccurred())
handlers, err := newConfigReaderRoleBindingHandler(logger, cl, commonTestUtils.GetScheme(), hco)
Expect(handlers).To(HaveLen(1))
Expect(err).To(BeNil())
res := handlers[0].ensure(req)
Expect(res.Err).NotTo(HaveOccurred())

foundRoleBinding := &rbacv1.RoleBinding{}
Expect(
Expand All @@ -1131,15 +1137,17 @@ var _ = Describe("CDI Operand", func() {
})

It("should update if labels are missing", func() {
expectedRoleBinding := NewConfigReaderRoleBindingForCR(hco, hco.Namespace)
expectedRoleBinding := NewCdiConfigReaderRoleBinding(hco)
expectedLabels := expectedRoleBinding.Labels
expectedRoleBinding.Labels = nil

cl := commonTestUtils.InitClient([]runtime.Object{hco, expectedRoleBinding})

handler := (*genericOperand)(newConfigReaderRoleBindingHandler(cl, commonTestUtils.GetScheme()))
res := handler.ensure(req)
Expect(res.Err).ToNot(HaveOccurred())
handlers, err := newConfigReaderRoleBindingHandler(logger, cl, commonTestUtils.GetScheme(), hco)
Expect(handlers).To(HaveLen(1))
Expect(err).To(BeNil())
res := handlers[0].ensure(req)
Expect(res.Err).NotTo(HaveOccurred())

foundRoleBinding := &rbacv1.RoleBinding{}
Expect(
Expand Down
75 changes: 75 additions & 0 deletions pkg/controller/operands/cmHandler.go
@@ -0,0 +1,75 @@
package operands

import (
"errors"
"reflect"

corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"sigs.k8s.io/controller-runtime/pkg/client"

hcov1beta1 "github.com/kubevirt/hyperconverged-cluster-operator/pkg/apis/hco/v1beta1"
"github.com/kubevirt/hyperconverged-cluster-operator/pkg/controller/common"
"github.com/kubevirt/hyperconverged-cluster-operator/pkg/util"
)

func newCmHandler(Client client.Client, Scheme *runtime.Scheme, required *corev1.ConfigMap) Operand {
h := &genericOperand{
Client: Client,
Scheme: Scheme,
crType: "ConfigMap",
removeExistingOwner: false,
hooks: &cmHooks{required: required},
}

return h
}

type cmHooks struct {
required *corev1.ConfigMap
}

func (h cmHooks) getFullCr(_ *hcov1beta1.HyperConverged) (client.Object, error) {
return h.required.DeepCopy(), nil
}

func (h cmHooks) getEmptyCr() client.Object {
return &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: h.required.Name,
},
}
}

func (h cmHooks) getObjectMeta(cr runtime.Object) *metav1.ObjectMeta {
return &cr.(*corev1.ConfigMap).ObjectMeta
}

func (h cmHooks) reset() { /* no implementation */ }

func (h cmHooks) updateCr(req *common.HcoRequest, Client client.Client, exists runtime.Object, _ runtime.Object) (bool, bool, error) {
found, ok := exists.(*corev1.ConfigMap)

if !ok {
return false, false, errors.New("can't convert to Configmap")
}

if !reflect.DeepEqual(found.Data, h.required.Data) ||
!reflect.DeepEqual(found.Labels, h.required.Labels) {
if req.HCOTriggered {
req.Logger.Info("Updating existing Configmap to new opinionated values", "name", h.required.Name)
} else {
req.Logger.Info("Reconciling an externally updated Configmap to its opinionated values", "name", h.required.Name)
}
util.DeepCopyLabels(&h.required.ObjectMeta, &found.ObjectMeta)
h.required.DeepCopyInto(found)
err := Client.Update(req.Ctx, found)
if err != nil {
return false, false, err
}
return true, !req.HCOTriggered, nil
}

return false, false, nil
}

0 comments on commit f06ce4b

Please sign in to comment.