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.

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

OSASINFRA-3492: openstack: leverage ORC to handle RHCOS image #5139

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 1 addition & 10 deletions cmd/nodepool/openstack/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,15 +69,6 @@ func (o *RawOpenStackPlatformCreateOptions) Validate() (*ValidatedOpenStackPlatf
return nil, fmt.Errorf("flavor is required")
}

// TODO(emilien): Remove that validation once we support using the image from the release payload.
// This will be possible when CAPO supports managing images in the OpenStack cluster:
// https://github.com/kubernetes-sigs/cluster-api-provider-openstack/pull/2130
// For 4.17 we might leave this as is and let the user provide the image name as
// we plan to deliver the OpenStack provider as a dev preview.
if o.ImageName == "" {
return nil, fmt.Errorf("image name is required")
}

additionalports, err := convertAdditionalPorts(o.AdditionalPorts)
if err != nil {
return nil, err
Expand All @@ -99,7 +90,7 @@ func BindOptions(opts *RawOpenStackPlatformCreateOptions, flags *pflag.FlagSet)

func bindCoreOptions(opts *RawOpenStackPlatformCreateOptions, flags *pflag.FlagSet) {
flags.StringVar(&opts.Flavor, "openstack-node-flavor", opts.Flavor, "The flavor to use for the nodepool (required)")
flags.StringVar(&opts.ImageName, "openstack-node-image-name", opts.ImageName, "The image name to use for the nodepool (required)")
flags.StringVar(&opts.ImageName, "openstack-node-image-name", opts.ImageName, "The image name to use for the nodepool (optional)")
flags.StringVar(&opts.AvailabityZone, "openstack-node-availability-zone", opts.AvailabityZone, "The availability zone to use for the nodepool (optional)")
flags.StringArrayVar(&opts.AdditionalPorts, "openstack-node-additional-port", opts.AdditionalPorts, fmt.Sprintf(`Specify additional port that should be attached to the nodes, the "network-id" field should point to an existing neutron network ID and the "vnic-type" is the type of the port to create, it can be specified multiple times to attach to multiple ports. Supported parameters: %s, example: "network-id:40a355cb-596d-495c-8766-419d98cadd57,vnic-type:direct"`, cmdutil.Supported(PortSpec{})))
}
Expand Down
13 changes: 1 addition & 12 deletions cmd/nodepool/openstack/create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,21 +15,10 @@ func TestRawOpenStackPlatformCreateOptions_Validate(t *testing.T) {
{
name: "should fail if flavor is missing",
input: RawOpenStackPlatformCreateOptions{
OpenStackPlatformOptions: &OpenStackPlatformOptions{
ImageName: "rhcos",
},
OpenStackPlatformOptions: &OpenStackPlatformOptions{},
},
expectedError: "flavor is required",
},
{
name: "should fail if image name is missing",
input: RawOpenStackPlatformCreateOptions{
OpenStackPlatformOptions: &OpenStackPlatformOptions{
Flavor: "flavor",
},
},
expectedError: "image name is required",
},
{
name: "should pass when AZ is provided",
input: RawOpenStackPlatformCreateOptions{
Expand Down
23 changes: 1 addition & 22 deletions docs/content/how-to/openstack/create-openstack-cluster.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,23 +88,6 @@ operator-755d587f44-lrtrq 1/1 Running 0 114s
operator-755d587f44-qj6pz 1/1 Running 0 114s
```

### Upload RHCOS image in OpenStack

For now, we need to manually push an RHCOS image that will be used when deploying the node pools
on OpenStack. In the [future](https://issues.redhat.com/browse/OSASINFRA-3492), the CAPI provider (CAPO) will handle the RHCOS image
lifecycle by using the image available in the chosen release payload.

Here is an example of how to upload an RHCOS image to OpenStack:

```shell
openstack image create --disk-format qcow2 --file rhcos-openstack.x86_64.qcow2 rhcos
```

!!! note

The `rhcos-openstack.x86_64.qcow2` file is the RHCOS image that was downloaded from the OpenShift mirror.
You can download the latest RHCOS image from the [Red Hat OpenShift Container Platform mirror](https://mirror.openshift.com/pub/openshift-v4/dependencies/rhcos/).

## Create a HostedCluster

Once all the [prerequisites](#prerequisites) are met, it is now possible to create a guest cluster.
Expand All @@ -124,9 +107,6 @@ export WORKER_COUNT="2"

export OS_CLOUD="openstack"

# Image name is the name of the image in OpenStack that was pushed in the previous step.
export IMAGE_NAME="rhcos"

# Flavor for the nodepool
export FLAVOR="m1.large"

Expand All @@ -153,7 +133,6 @@ hcp create cluster openstack \
--openstack-credentials-file $CLOUDS_YAML \
--openstack-ca-cert-file $CA_CERT_PATH \
--openstack-external-network-id $EXTERNAL_NETWORK_ID \
--openstack-node-image-name $IMAGE_NAME \
--openstack-node-flavor $FLAVOR
```

Expand Down Expand Up @@ -299,7 +278,7 @@ port for SR-IOV, with no port security and address pairs:
```shell
export NODEPOOL_NAME=$CLUSTER_NAME-extra-az
export WORKER_COUNT="2"
export IMAGE_NAME="rhcos"
export IMAGE_NAME="rhcos" # Pre-existing Glance image used for this NodePool
export FLAVOR="m1.xlarge"
export AZ="az1"
export SRIOV_NEUTRON_NETWORK_ID="f050901b-11bc-4a75-a553-878509255760"
Expand Down
11 changes: 11 additions & 0 deletions hypershift-operator/controllers/nodepool/capi.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,13 @@ func (c *CAPI) Reconcile(ctx context.Context) error {
return err
}

// Reconcile ORC resources
if nodePool.Spec.Platform.Type == hyperv1.OpenStackPlatform {
Copy link
Member Author

Choose a reason for hiding this comment

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

I don't know if this pattern will be accepted. Suggestions are welcome!

Copy link
Member Author

Choose a reason for hiding this comment

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

@bryan-cox I realized that we probably need to handle the OpenStack RHCOS image by Nodepool, since the release image can be overridden per Nodepool?

if err := c.reconcileORCResources(ctx); err != nil {
return err
}
}

// Reconcile (Platform)MachineTemplate.
template, mutateTemplate, _, err := c.machineTemplateBuilders()
if err != nil {
Expand Down Expand Up @@ -1375,3 +1382,7 @@ func (r *NodePoolReconciler) getMachinesForNodePool(ctx context.Context, nodePoo

return sortedByCreationTimestamp(machinesForNodePool), nil
}

func (c *CAPI) reconcileORCResources(ctx context.Context) error {
return openstack.ReconcileOpenStackImageCR(ctx, c.Client, c.CreateOrUpdate, c.hostedCluster, c.controlplaneNamespace, c.releaseImage)
}
4 changes: 2 additions & 2 deletions hypershift-operator/controllers/nodepool/kubevirt/kubevirt.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func defaultImage(releaseImage *releaseinfo.ReleaseImage) (string, string, error
return containerImage, split[1], nil
}

func unsupportedOpenstackDefaultImage(releaseImage *releaseinfo.ReleaseImage) (string, string, error) {
func UnsupportedOpenstackDefaultImage(releaseImage *releaseinfo.ReleaseImage) (string, string, error) {
Copy link
Member Author

Choose a reason for hiding this comment

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

We probably want to move that function to utils or something

arch, foundArch := releaseImage.StreamMetadata.Architectures["x86_64"]
if !foundArch {
return "", "", fmt.Errorf("couldn't find OS metadata for architecture %q", "x64_64")
Expand Down Expand Up @@ -101,7 +101,7 @@ func GetImage(nodePool *hyperv1.NodePool, releaseImage *releaseinfo.ReleaseImage

imageName, imageHash, err := defaultImage(releaseImage)
if err != nil && allowUnsupportedRHCOSVariants(nodePool) {
imageName, imageHash, err = unsupportedOpenstackDefaultImage(releaseImage)
imageName, imageHash, err = UnsupportedOpenstackDefaultImage(releaseImage)
if err != nil {
return nil, err
}
Expand Down
68 changes: 62 additions & 6 deletions hypershift-operator/controllers/nodepool/openstack/openstack.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,19 @@
package openstack

import (
"context"
"fmt"

hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"sigs.k8s.io/controller-runtime/pkg/client"

orc "github.com/k-orc/openstack-resource-controller/api/v1alpha1"
nodepoolkubevirt "github.com/openshift/hypershift/hypershift-operator/controllers/nodepool/kubevirt"
"github.com/openshift/hypershift/support/openstackutil"
"github.com/openshift/hypershift/support/releaseinfo"
"github.com/openshift/hypershift/support/upsert"
"k8s.io/utils/ptr"
capiopenstackv1beta1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1"
)
Expand All @@ -20,12 +28,9 @@ func MachineTemplateSpec(hcluster *hyperv1.HostedCluster, nodePool *hyperv1.Node
Name: ptr.To(nodePool.Spec.Platform.OpenStack.ImageName),
}
} else {
// TODO(emilien): Add support for using the image from the release payload.
// This will be possible when CAPO supports managing images in the OpenStack cluster:
// https://github.com/kubernetes-sigs/cluster-api-provider-openstack/pull/2130
// For 4.17 we might leave this as is and let the user provide the image name as
// we plan to deliver the OpenStack provider as a dev preview.
return nil, fmt.Errorf("image name is required")
openStackMachineTemplate.Template.Spec.Image.ImageRef = &capiopenstackv1beta1.ResourceReference{
Name: "rhcos-" + hcluster.Name,
}
}

// TODO: add support for BYO network/subnet
Expand Down Expand Up @@ -72,3 +77,54 @@ func MachineTemplateSpec(hcluster *hyperv1.HostedCluster, nodePool *hyperv1.Node
}
return openStackMachineTemplate, nil
}

func ReconcileOpenStackImageCR(ctx context.Context, client client.Client, createOrUpdate upsert.CreateOrUpdateFN, hcluster *hyperv1.HostedCluster, controlPlaneNamespace string, release *releaseinfo.ReleaseImage) error {
openStackImage := orc.Image{
ObjectMeta: metav1.ObjectMeta{
Name: "rhcos-" + hcluster.Name,
Copy link
Member Author

Choose a reason for hiding this comment

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

TODO: name

Namespace: controlPlaneNamespace,
},
Spec: orc.ImageSpec{},
}

if _, err := createOrUpdate(ctx, client, &openStackImage, func() error {
err := reconcileOpenStackImageSpec(ctx, client, hcluster, &openStackImage.Spec, release)
if err != nil {
return err
}
return nil
}); err != nil {
return err
}
return nil
}

func reconcileOpenStackImageSpec(ctx context.Context, client client.Client, hcluster *hyperv1.HostedCluster, openStackImageSpec *orc.ImageSpec, release *releaseinfo.ReleaseImage) error {
imageURL, imageHash, err := nodepoolkubevirt.UnsupportedOpenstackDefaultImage(release)
if err != nil {
return fmt.Errorf("failed to lookup RHCOS image: %w", err)
}

openStackImageSpec.CloudCredentialsRef = orc.CloudCredentialsReference{
SecretName: hcluster.Spec.Platform.OpenStack.IdentityRef.Name,
CloudName: hcluster.Spec.Platform.OpenStack.IdentityRef.CloudName,
}

openStackImageSpec.Resource = &orc.ImageResourceSpec{
// THIS IS NOT GOOD, NEEDS TO BE FIXED (something related to the image itself should be used, like version)
Name: "rhcos-" + hcluster.Name,
Copy link
Member Author

Choose a reason for hiding this comment

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

TODO: name

Content: &orc.ImageContent{
DiskFormat: "qcow2",
Download: &orc.ImageContentSourceDownload{
URL: imageURL,
Decompress: ptr.To(orc.ImageCompressionGZ),
Hash: &orc.ImageHash{
Algorithm: "sha256",
Value: imageHash,
},
},
},
}

return nil
}
1 change: 0 additions & 1 deletion test/e2e/e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -563,7 +563,6 @@ func (p *options) DefaultOpenStackOptions() hypershiftopenstack.RawCreateOptions
NodePoolOpts: &openstacknodepool.RawOpenStackPlatformCreateOptions{
OpenStackPlatformOptions: &openstacknodepool.OpenStackPlatformOptions{
Flavor: p.configurableClusterOptions.OpenStackNodeFlavor,
ImageName: p.configurableClusterOptions.OpenStackNodeImageName,
AvailabityZone: p.configurableClusterOptions.OpenStackNodeAvailabilityZone,
},
},
Expand Down
124 changes: 124 additions & 0 deletions test/e2e/nodepool_osp_image_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
//go:build e2e
// +build e2e

package e2e

import (
"context"
"fmt"
"testing"

"github.com/google/go-cmp/cmp"
hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1"
"github.com/openshift/hypershift/hypershift-operator/controllers/manifests"
e2eutil "github.com/openshift/hypershift/test/e2e/util"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/utils/ptr"
capiopenstackv1alpha1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha1"
capiopenstackv1beta1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1"
capiv1 "sigs.k8s.io/cluster-api/api/v1beta1"
"sigs.k8s.io/cluster-api/util"
crclient "sigs.k8s.io/controller-runtime/pkg/client"
)

type OpenStackImageTest struct {
DummyInfraSetup
ctx context.Context
managementClient crclient.Client
hostedCluster *hyperv1.HostedCluster
hostedControlPlaneNamespace string
}

func NewOpenStackImageTest(ctx context.Context, mgmtClient crclient.Client, hostedCluster *hyperv1.HostedCluster) *OpenStackImageTest {
return &OpenStackImageTest{
ctx: ctx,
hostedCluster: hostedCluster,
managementClient: mgmtClient,
hostedControlPlaneNamespace: manifests.HostedControlPlaneNamespace(hostedCluster.Namespace, hostedCluster.Name),
}
}

func (o OpenStackImageTest) Setup(t *testing.T) {
t.Log("Starting test OpenStackImageTest")

if globalOpts.Platform != hyperv1.OpenStackPlatform {
t.Skip("test only supported on platform OpenStack")
}

if globalOpts.configurableClusterOptions.OpenStackNodeImageName == "" {
t.Skip("OpenStack image name not provided, skipping test")
}
}

func (o OpenStackImageTest) Run(t *testing.T, nodePool hyperv1.NodePool, _ []corev1.Node) {
np := &hyperv1.NodePool{}
e2eutil.EventuallyObject(t, o.ctx, "NodePool to have image configured",
func(ctx context.Context) (*hyperv1.NodePool, error) {
err := o.managementClient.Get(ctx, util.ObjectKey(&nodePool), np)
return np, err
},
[]e2eutil.Predicate[*hyperv1.NodePool]{
func(nodePool *hyperv1.NodePool) (done bool, reasons string, err error) {
want, got := hyperv1.OpenStackPlatform, nodePool.Spec.Platform.Type
return want == got, fmt.Sprintf("expected NodePool to have platform %s, got %s", want, got), nil
},
func(pool *hyperv1.NodePool) (done bool, reasons string, err error) {
diff := cmp.Diff(globalOpts.configurableClusterOptions.OpenStackNodeImageName, ptr.Deref(np.Spec.Platform.OpenStack, hyperv1.OpenStackNodePoolPlatform{}).ImageName)
return diff == "", fmt.Sprintf("incorrect image name: %v", diff), nil
},
},
)

e2eutil.EventuallyObjects(t, o.ctx, "OpenStackServers to be created with the correct image",
func(ctx context.Context) ([]*capiopenstackv1beta1.OpenStackMachine, error) {
list := &capiopenstackv1beta1.OpenStackMachineList{}
err := o.managementClient.List(ctx, list, crclient.InNamespace(o.hostedControlPlaneNamespace), crclient.MatchingLabels{capiv1.MachineDeploymentNameLabel: nodePool.Name})
oms := make([]*capiopenstackv1beta1.OpenStackMachine, len(list.Items))
for i := range list.Items {
oms[i] = &list.Items[i]
}
return oms, err
},
[]e2eutil.Predicate[[]*capiopenstackv1beta1.OpenStackMachine]{
func(machines []*capiopenstackv1beta1.OpenStackMachine) (done bool, reasons string, err error) {
return len(machines) == int(*nodePool.Spec.Replicas), fmt.Sprintf("expected %d OpenStackMachines, got %d", *nodePool.Spec.Replicas, len(machines)), nil
},
},
[]e2eutil.Predicate[*capiopenstackv1beta1.OpenStackMachine]{
func(machine *capiopenstackv1beta1.OpenStackMachine) (done bool, reasons string, err error) {
server := &capiopenstackv1alpha1.OpenStackServer{}
err = o.managementClient.Get(o.ctx, crclient.ObjectKey{Name: machine.Name, Namespace: o.hostedControlPlaneNamespace}, server)
if err != nil {
return false, "", err
}
if server.Spec.Image.Filter != nil && *server.Spec.Image.Filter.Name != globalOpts.configurableClusterOptions.OpenStackNodeImageName {
return false, fmt.Sprintf("expected image name %s, got %s", globalOpts.configurableClusterOptions.OpenStackNodeImageName, *server.Spec.Image.Filter.Name), nil
}
return true, "", nil
},
},
)
}

func (o OpenStackImageTest) BuildNodePoolManifest(defaultNodepool hyperv1.NodePool) (*hyperv1.NodePool, error) {
nodePool := &hyperv1.NodePool{
ObjectMeta: metav1.ObjectMeta{
Name: o.hostedCluster.Name + "-" + "test-osp-image",
Namespace: o.hostedCluster.Namespace,
},
}
defaultNodepool.Spec.DeepCopyInto(&nodePool.Spec)

nodePool.Spec.Replicas = &oneReplicas
nodePool.Spec.Platform.OpenStack.ImageName = globalOpts.configurableClusterOptions.OpenStackNodeImageName
return nodePool, nil
}

func (o OpenStackImageTest) SetupInfra(t *testing.T) error {
return nil
}

func (o OpenStackImageTest) TeardownInfra(t *testing.T) error {
return nil
}
4 changes: 4 additions & 0 deletions test/e2e/nodepool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,10 @@ func TestNodePool(t *testing.T) {
name: "OpenStackAZTest",
test: NewOpenStackAZTest(ctx, mgtClient, hostedCluster),
},
{
name: "OpenStackImageTest",
test: NewOpenStackImageTest(ctx, mgtClient, hostedCluster),
},
{
name: "TestMirrorConfigs",
test: NewMirrorConfigsTest(ctx, mgtClient, hostedCluster, hostedClusterClient),
Expand Down