Deploy common-instancetypes v0.0.1-rc #453

lyarwood · 2022-11-18T19:22:50Z

What this PR does / why we need it:

This initial change starts to simply deploy the initial v0.0.1-rc bundle
of VirtualMachineCluster{Instancetypes,Preferences} resources. These
resources are deployed under both OpenShift and non-OpenShift based
environments, at present this it the only functionality enabled under
the latter.

Future changes will introduce support for removing deprecated resources
and support for deploying resources from a repo URI.

$ env | grep KUBEVIRT
KUBEVIRT_PROVIDER=k8s-1.24
KUBEVIRT_MEMORY=16384
KUBEVIRTCI_RUNTIME=docker
$ make cluster-down && make cluster-up &&  make cluster-sync
[..]
$ ./cluster/kubectl.sh get virtualmachineclusterpreferences -o json | jq '.items | .[] | .metadata.name'
selecting docker as container runtime
"alpine"
"centos.7"
"centos.7.desktop"
"centos.8.stream"
"centos.8.stream.desktop"
"centos.9.stream"
"centos.9.stream.desktop"
"cirros"
"fedora.35"
"fedora.36"
"rhel.7"
"rhel.7.desktop"
"rhel.8"
"rhel.8.desktop"
"rhel.9"
"rhel.9.desktop"
"ubuntu.18.04"
"ubuntu.20.04"
"ubuntu.22.04"
"windows.10"
"windows.10.virtio"
"windows.11"
"windows.11.virtio"
"windows.2k12"
"windows.2k12.virtio"
"windows.2k16"
"windows.2k16.virtio"
"windows.2k19"
"windows.2k19.virtio"
"windows.2k22"
"windows.2k22.virtio"
$ ./cluster/kubectl.sh get virtualmachineclusterinstancetypes -o json | jq '.items | .[] | .metadata.name'
selecting docker as container runtime
"highperformance.large"
"highperformance.medium"
"highperformance.small"
"server.large"
"server.medium"
"server.micro"
"server.small"
"server.tiny"

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Release note:

`VirtualMachineClusterInstancetypes` and `VirtualMachineClusterPreferences` generated by the [`v0.0.1-rc`](https://github.com/kubevirt/common-instancetypes/releases/tag/v0.0.1-rc) release of the [common-instancetypes](https://github.com/kubevirt/common-instancetypes) project are now installed by default by the operator.

openshift-ci · 2022-11-18T19:22:53Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

lyarwood · 2022-11-21T17:22:00Z

Marked this as ready to review but it still obviously needs tests etc, just looking for some initial reviews on direction.

0xFelix

On nit, otherwise lgtm!

config/rbac/role.yaml

data/olm-catalog/ssp-operator.clusterserviceversion.yaml

data/common-instancetypes-bundle/common-clusterpreferences-bundle.yaml

internal/operands/common-instancetypes/reconcile.go

0xFelix · 2022-12-08T13:15:27Z

internal/operands/common-instancetypes/reconcile.go

+				ClusterResource(clusterInstancetype).
+				WithAppLabels(operandName, operandComponent).
+				UpdateFunc(func(newRes, foundRes client.Object) {
+					foundRes.(*instancetypev1alpha2.VirtualMachineClusterInstancetype).Spec = newRes.(*instancetypev1alpha2.VirtualMachineClusterInstancetype).Spec


Should the casts be checked?

internal/operands/common-instancetypes/reconcile.go

internal/operands/common-instancetypes/reconsile_test.go

internal/operands/common-instancetypes/reconcile.go

internal/operands/common-instancetypes/reconsile_test.go

This initial change starts to simply deploy the v0.0.1-rc bundle of VirtualMachineCluster{Instancetypes,Preferences} resources. These resources are deployed under both OpenShift and non-OpenShift environments. At present this is the only functionality enabled within the operator when deployed under non-OpenShift environments. Future changes will introduce support for removing deprecated resources and for deploying resources from a kustomize repo URI. Signed-off-by: Lee Yarwood <[email protected]>

sonarqubecloud · 2022-12-08T17:14:06Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

0xFelix

Only one nit, otherwise LGTM! Thank you.

/approve
/lgtm

0xFelix · 2022-12-09T14:14:01Z

internal/operands/common-instancetypes/reconcile.go

+}
+
+func New(instancetypeBundlePath, preferenceBundlePath string) (operands.Operand, error) {
+	virtualMachineClusterInstancetypes, err := fetchClusterResources[instancetypev1alpha2.VirtualMachineClusterInstancetype](instancetypeBundlePath)


I wonder if there's a way to make the import name less clunky.

kubevirt-bot · 2022-12-09T14:35:53Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 0xFelix

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [0xFelix]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

0xFelix · 2022-12-09T14:36:06Z

/retest

akrejcir

I'm sorry I didn't look at this before it was merged.
I have some comments, can you implement them in another PR?
And also functional tests are missing.

akrejcir · 2022-12-09T16:58:39Z

internal/operands/common-instancetypes/reconcile.go

+}
+
+func (c *commonInstancetypes) RequiredCrds() []string {
+	return nil


This operand requires these CRDs:

virtualmachineclusterinstancetypes.instancetype.kubevirt.io virtualmachineclusterpreferences.instancetype.kubevirt.io

akrejcir · 2022-12-09T16:59:24Z

internal/operands/common-instancetypes/reconcile.go

+}
+
+func (c *commonInstancetypes) Cleanup(request *common.Request) ([]common.CleanupResult, error) {
+	return nil, nil


This method must remove the created cluster resources.

The example from common-templates only drops deprecated templates so I wasn't sure what the expected behaviour here actually was:

ssp-operator/internal/operands/common-templates/reconcile.go

Lines 116 to 137 in d95ccb8

func (c *commonTemplates) Cleanup(request *common.Request) ([]common.CleanupResult, error) {

var objects []client.Object

namespace := request.Instance.Spec.CommonTemplates.Namespace

deprecatedTemplates, err := getDeprecatedTemplates(request)

if err != nil {

return nil, err

}

for _, obj := range deprecatedTemplates.Items {

obj.ObjectMeta.Namespace = namespace

objects = append(objects, &obj)

}

for index := range c.templatesBundle {

c.templatesBundle[index].ObjectMeta.Namespace = namespace

objects = append(objects, &c.templatesBundle[index])

}

return common.DeleteAll(request, objects...)

}

I'll remove them all now in a follow up.

akrejcir · 2022-12-09T17:00:28Z

internal/operands/common-instancetypes/reconcile.go

+				UpdateFunc(func(newRes, foundRes client.Object) {
+					foundRes.(*instancetypev1alpha2.VirtualMachineClusterInstancetype).Spec = newRes.(*instancetypev1alpha2.VirtualMachineClusterInstancetype).Spec
+				}).
+				ImmutableSpec(func(resource client.Object) interface{} {


The VirtualMachineClusterInstancetype is not immutable, so this method should not be called.

Ah apologies I misunderstood things here, I'll drop it.

akrejcir · 2022-12-09T17:00:55Z

internal/operands/common-instancetypes/reconcile.go

+				UpdateFunc(func(newRes, foundRes client.Object) {
+					foundRes.(*instancetypev1alpha2.VirtualMachineClusterPreference).Spec = newRes.(*instancetypev1alpha2.VirtualMachineClusterPreference).Spec
+				}).
+				ImmutableSpec(func(resource client.Object) interface{} {


Similarly, preference is not immutable, so this method should not be called.

kubevirt-bot added release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Nov 18, 2022

kubevirt-bot requested review from 0xFelix and akrejcir November 18, 2022 19:22

kubevirt-bot added the size/XXL label Nov 18, 2022

lyarwood changed the title ~~Deploy common instancetypes~~ WIP Deploy common instancetypes Nov 18, 2022

lyarwood force-pushed the deploy-common-instancetypes branch from b32febe to e931ce4 Compare November 21, 2022 15:55

lyarwood changed the title ~~WIP Deploy common instancetypes~~ Deploy common-instancetypes v0.0.1-rc Nov 21, 2022

lyarwood changed the title ~~Deploy common-instancetypes v0.0.1-rc~~ Deploy common-instancetypes v0.0.1-rc Nov 21, 2022

kubevirt-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Nov 21, 2022

lyarwood marked this pull request as ready for review November 21, 2022 15:59

kubevirt-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 21, 2022

lyarwood force-pushed the deploy-common-instancetypes branch from e931ce4 to 5695c42 Compare November 21, 2022 17:35

0xFelix reviewed Nov 24, 2022

View reviewed changes

config/rbac/role.yaml Show resolved Hide resolved

data/olm-catalog/ssp-operator.clusterserviceversion.yaml Show resolved Hide resolved

lyarwood mentioned this pull request Nov 28, 2022

Introduce support for deployments via kubevirtci and kubevirt #452

Merged

lyarwood force-pushed the deploy-common-instancetypes branch from 5695c42 to e65b4a0 Compare November 28, 2022 20:26

kubevirt-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 7, 2022

lyarwood force-pushed the deploy-common-instancetypes branch from e65b4a0 to d34d127 Compare December 7, 2022 16:25

kubevirt-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 7, 2022

lyarwood force-pushed the deploy-common-instancetypes branch from d34d127 to 9aff1b9 Compare December 7, 2022 16:32

0xFelix reviewed Dec 8, 2022

View reviewed changes

lyarwood force-pushed the deploy-common-instancetypes branch from 9aff1b9 to b521a26 Compare December 8, 2022 17:13

0xFelix approved these changes Dec 9, 2022

View reviewed changes

kubevirt-bot assigned 0xFelix Dec 9, 2022

kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Dec 9, 2022

kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 9, 2022

kubevirt-bot merged commit d95ccb8 into kubevirt:master Dec 9, 2022

akrejcir reviewed Dec 9, 2022

View reviewed changes

lyarwood mentioned this pull request Dec 12, 2022

Follow ups for common-instancetypes deployment #462

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deploy common-instancetypes v0.0.1-rc #453

Deploy common-instancetypes v0.0.1-rc #453

lyarwood commented Nov 18, 2022 •

edited

Loading

openshift-ci bot commented Nov 18, 2022

lyarwood commented Nov 21, 2022

0xFelix left a comment

0xFelix Dec 8, 2022

sonarqubecloud bot commented Dec 8, 2022

0xFelix left a comment

0xFelix Dec 9, 2022

kubevirt-bot commented Dec 9, 2022

0xFelix commented Dec 9, 2022

akrejcir left a comment

akrejcir Dec 9, 2022

akrejcir Dec 9, 2022

lyarwood Dec 12, 2022 •

edited

Loading

akrejcir Dec 9, 2022

lyarwood Dec 12, 2022

akrejcir Dec 9, 2022

	func (c commonTemplates) Cleanup(request common.Request) ([]common.CleanupResult, error) {
	var objects []client.Object
	namespace := request.Instance.Spec.CommonTemplates.Namespace

	deprecatedTemplates, err := getDeprecatedTemplates(request)
	if err != nil {
	return nil, err
	}

	for _, obj := range deprecatedTemplates.Items {
	obj.ObjectMeta.Namespace = namespace
	objects = append(objects, &obj)
	}

	for index := range c.templatesBundle {
	c.templatesBundle[index].ObjectMeta.Namespace = namespace
	objects = append(objects, &c.templatesBundle[index])
	}

	return common.DeleteAll(request, objects...)
	}

Deploy common-instancetypes v0.0.1-rc #453

Deploy common-instancetypes v0.0.1-rc #453

Conversation

lyarwood commented Nov 18, 2022 • edited Loading

openshift-ci bot commented Nov 18, 2022

lyarwood commented Nov 21, 2022

0xFelix left a comment

Choose a reason for hiding this comment

0xFelix Dec 8, 2022

Choose a reason for hiding this comment

sonarqubecloud bot commented Dec 8, 2022

0xFelix left a comment

Choose a reason for hiding this comment

0xFelix Dec 9, 2022

Choose a reason for hiding this comment

kubevirt-bot commented Dec 9, 2022

0xFelix commented Dec 9, 2022

akrejcir left a comment

Choose a reason for hiding this comment

akrejcir Dec 9, 2022

Choose a reason for hiding this comment

akrejcir Dec 9, 2022

Choose a reason for hiding this comment

lyarwood Dec 12, 2022 • edited Loading

Choose a reason for hiding this comment

akrejcir Dec 9, 2022

Choose a reason for hiding this comment

lyarwood Dec 12, 2022

Choose a reason for hiding this comment

akrejcir Dec 9, 2022

Choose a reason for hiding this comment

lyarwood commented Nov 18, 2022 •

edited

Loading

lyarwood Dec 12, 2022 •

edited

Loading