Skip to content

Use community gallery for default VM images #5167

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

Merged
merged 2 commits into from
Oct 30, 2024
Merged

Use community gallery for default VM images #5167

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
02bc67d
Use community gallery for default VM images
mboersma Sep 6, 2024
f05ef21
Revert "Skip AKS BYO nodepool test"
mboersma Oct 8, 2024
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
2 changes: 1 addition & 1 deletion Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
ARG ARCH

# Build the manager binary
FROM golang:1.22 as builder
FROM golang:1.22 AS builder
WORKDIR /workspace

# Run this with docker build --build_arg $(go env GOPROXY) to override the goproxy
Expand Down
2 changes: 1 addition & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ GINKGO_VER := $(shell go list -m -f '{{.Version}}' github.com/onsi/ginkgo/v2)
GINKGO_BIN := ginkgo
GINKGO := $(TOOLS_BIN_DIR)/$(GINKGO_BIN)-$(GINKGO_VER)

KUBECTL_VER := v1.28.4
KUBECTL_VER := v1.29.10
KUBECTL_BIN := kubectl
KUBECTL := $(TOOLS_BIN_DIR)/$(KUBECTL_BIN)-$(KUBECTL_VER)

Expand Down
4 changes: 2 additions & 2 deletions Tiltfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ def validate_auth():

tilt_helper_dockerfile_header = """
# Tilt image
FROM golang:1.22 as tilt-helper
FROM golang:1.22 AS tilt-helper
# Support live reloading with Tilt
RUN wget --output-document /restart.sh --quiet https://raw.githubusercontent.com/windmilleng/rerun-process-wrapper/master/restart.sh && \
wget --output-document /start.sh --quiet https://raw.githubusercontent.com/windmilleng/rerun-process-wrapper/master/start.sh && \
Expand All @@ -138,7 +138,7 @@ RUN wget --output-document /restart.sh --quiet https://raw.githubusercontent.com
"""

tilt_dockerfile_header = """
FROM gcr.io/distroless/base:debug as tilt
FROM gcr.io/distroless/base:debug AS tilt
WORKDIR /tilt
RUN ["/busybox/chmod", "0777", "."]
COPY --from=tilt-helper /process.txt .
Expand Down
4 changes: 2 additions & 2 deletions api/v1beta1/azuremachine_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ type AzureMachineSpec struct {
FailureDomain *string `json:"failureDomain,omitempty"`

// Image is used to provide details of an image to use during VM creation.
// If image details are omitted the image will default the Azure Marketplace "capi" offer,
// which is based on Ubuntu.
// If image details are omitted, the default is to use an Azure Compute Gallery Image
// from CAPZ's community gallery.
// +kubebuilder:validation:nullable
// +optional
Image *Image `json:"image,omitempty"`
Expand Down
2 changes: 1 addition & 1 deletion azure/converters/vmss.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ func GetOrchestrationMode(modeType infrav1.OrchestrationModeType) armcompute.Orc
return armcompute.OrchestrationModeUniform
}

// IDImageRefToImage converts an ID to a infrav1.Image with ComputerGallery set or ID, depending on the structure of the ID.
// IDImageRefToImage converts an ID to a infrav1.Image with ComputeGallery set or ID, depending on the structure of the ID.
func IDImageRefToImage(id string) infrav1.Image {
// compute gallery image
if ok, params := getParams(RegExpStrComputeGalleryID, id); ok {
Expand Down
14 changes: 6 additions & 8 deletions azure/defaults.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,12 @@ const (
)

const (
// DefaultImageOfferID is the default Azure Marketplace offer ID.
DefaultImageOfferID = "capi"
// DefaultWindowsImageOfferID is the default Azure Marketplace offer ID for Windows.
DefaultWindowsImageOfferID = "capi-windows"
// DefaultImagePublisherID is the default Azure Marketplace publisher ID.
DefaultImagePublisherID = "cncf-upstream"
// LatestVersion is the image version latest.
LatestVersion = "latest"
// DefaultPublicGalleryName is the default Azure compute gallery.
DefaultPublicGalleryName = "ClusterAPI-f72ceb4f-5159-4c26-a0fe-2ea738f0d019"
Copy link
Contributor

Choose a reason for hiding this comment

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

Parroting @jsturtevant, are you planning to open a k8s.io PR to add this new gallery to the Terraform?

Copy link
Contributor Author

@mboersma mboersma Oct 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yes indeed, but I haven't started that yet. That's the third piece of the puzzle, in addition to this and kubernetes-sigs/image-builder#1578.

Edit: terraform PR is at kubernetes/k8s.io#7461

Copy link
Contributor

Choose a reason for hiding this comment

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

does this gallery name relate to anything in here?

kubernetes/k8s.io#7461

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesn't--I don't think there's a way to recreate a community gallery with the same unique name if it were to be deleted. It gets created for you, starting with your specified prefix, when you actually share the gallery to the world.

So AFAICT it isn't / shouldn't be part of the terraform for the gallery.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This implies if the gallery were to be deleted, we'd have to change code to point to a new gallery with the same name prefix. That's not great. I can't add a "Delete" lock on the resource, because that has the side effect of disallowing new image definitions to be added or old versions to be deleted, etc.

Copy link
Contributor

Choose a reason for hiding this comment

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

So once we apply the terraform we will change the value here to reflect the unique name that it gets?

Copy link
Contributor Author

@mboersma mboersma Oct 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

It's already got a unique name by virtue of me recreating everything in the CNCF community sub and then reverse-engineering the terraform. AFAICT, the unique name it gets is stored under its sharingProfile which isn't in the terraform I generated...but maybe can be added?

% az sig show -g cluster-api-gallery --gallery-name community_gallery -o yaml
description: Shared image gallery for Cluster API Provider Azure
id: /subscriptions/46678f10-4bbb-447e-98e8-d2829589f2d8/resourceGroups/cluster-api-gallery/providers/Microsoft.Compute/galleries/community_gallery
identifier:
  uniqueName: 46678f10-4bbb-447e-98e8-d2829589f2d8-COMMUNITY_GALLERY
location: northcentralus
name: community_gallery
provisioningState: Succeeded
resourceGroup: cluster-api-gallery
sharingProfile:
  communityGalleryInfo:
    communityGalleryEnabled: true
    eula: https://raw.githubusercontent.com/kubernetes-sigs/cluster-api-provider-azure/main/LICENSE
    publicNamePrefix: ClusterAPI
    publicNames:
    - ClusterAPI-f72ceb4f-5159-4c26-a0fe-2ea738f0d019
    publisherContact: [email protected]
    publisherUri: https://github.com/kubernetes-sigs/cluster-api-provider-azure
  groups: null
  permissions: Community
sharingStatus: null
softDeletePolicy: null
tags:
  DO-NOT-DELETE: UpstreamInfra
  DateCreated: 10/24/2024
  creationTimestamp: '2024-10-24T17:36:37Z'
  jobName: image-builder-sig-ubuntu-2404
type: Microsoft.Compute/galleries

Copy link
Contributor Author

@mboersma mboersma Oct 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

But from the point of view of CAPZ's code here, this is the unique identifier we need to access images. It shouldn't change except in a disaster recovery case where we have to set up a new community gallery.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could try to refactor this away from being a constant into something created at runtime that the user could override if needed. (Or I can follow up with that change after this merges.)

Copy link
Contributor

Choose a reason for hiding this comment

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

If users can set the gallery name in the API, I don't think we need a separate toggle to change the default for now.

// DefaultLinuxGalleryImageName is the default Linux community gallery image definition.
DefaultLinuxGalleryImageName = "capi-ubun2-2404"
// DefaultWindowsGalleryImageName is the default Windows community gallery image definition.
DefaultWindowsGalleryImageName = "capi-win-2019-containerd"
)

const (
Expand Down
2 changes: 1 addition & 1 deletion azure/scope/machine.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -725,7 +725,7 @@ func (m *MachineScope) GetVMImage(ctx context.Context) (*infrav1.Image, error) {
}

log.Info("No image specified for machine, using default Linux Image", "machine", m.AzureMachine.GetName())
return svc.GetDefaultUbuntuImage(ctx, m.Location(), ptr.Deref(m.Machine.Spec.Version, ""))
return svc.GetDefaultLinuxImage(ctx, m.Location(), ptr.Deref(m.Machine.Spec.Version, ""))
}

// SetSubnetName defaults the AzureMachine subnet name to the name of one the subnets with the machine role when there is only one of them.
Expand Down
107 changes: 8 additions & 99 deletions azure/scope/machine_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ import (
"sigs.k8s.io/cluster-api-provider-azure/azure/services/resourceskus"
"sigs.k8s.io/cluster-api-provider-azure/azure/services/roleassignments"
"sigs.k8s.io/cluster-api-provider-azure/azure/services/virtualmachineimages"
"sigs.k8s.io/cluster-api-provider-azure/azure/services/virtualmachineimages/mock_virtualmachineimages"
"sigs.k8s.io/cluster-api-provider-azure/azure/services/vmextensions"
)

Expand Down Expand Up @@ -1538,7 +1537,7 @@ func TestMachineScope_GetVMImage(t *testing.T) {
clusterMock.EXPECT().SubscriptionID().AnyTimes()
clusterMock.EXPECT().CloudEnvironment().AnyTimes()
clusterMock.EXPECT().Token().Return(&azidentity.DefaultAzureCredential{}).AnyTimes()
svc := virtualmachineimages.Service{Client: mock_virtualmachineimages.NewMockClient(mockCtrl)}
svc := virtualmachineimages.Service{}

tests := []struct {
name string
Expand All @@ -1547,7 +1546,7 @@ func TestMachineScope_GetVMImage(t *testing.T) {
expectedErr string
}{
{
name: "returns AzureMachine image is found if present in the AzureMachine spec",
name: "returns AzureMachine image if present in the AzureMachine spec",
machineScope: MachineScope{
AzureMachine: &infrav1.AzureMachine{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -1566,7 +1565,7 @@ func TestMachineScope_GetVMImage(t *testing.T) {
expectedErr: "",
},
{
name: "if no image is specified and os specified is windows with version below 1.22, returns windows dockershim image",
name: "if no image is specified and os specified is windows, returns windows containerd image",
machineScope: MachineScope{
Machine: &clusterv1.Machine{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -1589,42 +1588,13 @@ func TestMachineScope_GetVMImage(t *testing.T) {
ClusterScoper: clusterMock,
},
want: func() *infrav1.Image {
image, _ := svc.GetDefaultWindowsImage(context.TODO(), "", "1.20.1", "dockershim", "")
image, _ := svc.GetDefaultWindowsImage(context.TODO(), "", "1.20.1", "containerd", "")
return image
}(),
expectedErr: "",
},
{
name: "if no image is specified and os specified is windows with version is 1.22+ with no annotation, returns windows containerd image",
machineScope: MachineScope{
Machine: &clusterv1.Machine{
ObjectMeta: metav1.ObjectMeta{
Name: "machine-name",
},
Spec: clusterv1.MachineSpec{
Version: ptr.To("1.22.1"),
},
},
AzureMachine: &infrav1.AzureMachine{
ObjectMeta: metav1.ObjectMeta{
Name: "machine-name",
},
Spec: infrav1.AzureMachineSpec{
OSDisk: infrav1.OSDisk{
OSType: azure.WindowsOS,
},
},
},
ClusterScoper: clusterMock,
},
want: func() *infrav1.Image {
image, _ := svc.GetDefaultWindowsImage(context.TODO(), "", "1.22.1", "containerd", "")
return image
}(),
expectedErr: "",
},
{
name: "if no image is specified and os specified is windows with version is 1.22+ with annotation dockershim, returns windows dockershim image",
name: "if no image is specified and os specified is windows with annotation dockershim, returns error",
machineScope: MachineScope{
Machine: &clusterv1.Machine{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -1653,71 +1623,10 @@ func TestMachineScope_GetVMImage(t *testing.T) {
image, _ := svc.GetDefaultWindowsImage(context.TODO(), "", "1.22.1", "dockershim", "")
return image
}(),
expectedErr: "",
},
{
name: "if no image is specified and os specified is windows with version is less and 1.22 with annotation dockershim, returns windows dockershim image",
machineScope: MachineScope{
Machine: &clusterv1.Machine{
ObjectMeta: metav1.ObjectMeta{
Name: "machine-name",
},
Spec: clusterv1.MachineSpec{
Version: ptr.To("1.21.1"),
},
},
AzureMachine: &infrav1.AzureMachine{
ObjectMeta: metav1.ObjectMeta{
Name: "machine-name",
Annotations: map[string]string{
"runtime": "dockershim",
},
},
Spec: infrav1.AzureMachineSpec{
OSDisk: infrav1.OSDisk{
OSType: azure.WindowsOS,
},
},
},
ClusterScoper: clusterMock,
},
want: func() *infrav1.Image {
image, _ := svc.GetDefaultWindowsImage(context.TODO(), "", "1.21.1", "dockershim", "")
return image
}(),
expectedErr: "",
},
{
name: "if no image is specified and os specified is windows with version is less and 1.22 with annotation containerd, returns error",
machineScope: MachineScope{
Machine: &clusterv1.Machine{
ObjectMeta: metav1.ObjectMeta{
Name: "machine-name",
},
Spec: clusterv1.MachineSpec{
Version: ptr.To("1.21.1"),
},
},
AzureMachine: &infrav1.AzureMachine{
ObjectMeta: metav1.ObjectMeta{
Name: "machine-name",
Annotations: map[string]string{
"runtime": "containerd",
},
},
Spec: infrav1.AzureMachineSpec{
OSDisk: infrav1.OSDisk{
OSType: azure.WindowsOS,
},
},
},
ClusterScoper: clusterMock,
},
want: nil,
expectedErr: "containerd image only supported in 1.22+",
expectedErr: "unsupported runtime dockershim",
},
{
name: "if no image is specified and os specified is windows with windowsServerVersion annotation set to 2019, retrurns 2019 image",
name: "if no image is specified and os specified is windows with windowsServerVersion annotation set to 2019, returns error",
machineScope: MachineScope{
Machine: &clusterv1.Machine{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -1799,7 +1708,7 @@ func TestMachineScope_GetVMImage(t *testing.T) {
ClusterScoper: clusterMock,
},
want: func() *infrav1.Image {
image, _ := svc.GetDefaultUbuntuImage(context.TODO(), "", "1.20.1")
image, _ := svc.GetDefaultLinuxImage(context.TODO(), "", "1.20.1")
return image
}(),
expectedErr: "",
Expand Down
2 changes: 1 addition & 1 deletion azure/scope/machinepool.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -800,7 +800,7 @@ func (m *MachinePoolScope) GetVMImage(ctx context.Context) (*infrav1.Image, erro
log.V(4).Info("No image specified for machine, using default Windows Image", "machine", m.MachinePool.GetName(), "runtime", runtime, "windowsServerVersion", windowsServerVersion)
defaultImage, err = svc.GetDefaultWindowsImage(ctx, m.Location(), ptr.Deref(m.MachinePool.Spec.Template.Spec.Version, ""), runtime, windowsServerVersion)
} else {
defaultImage, err = svc.GetDefaultUbuntuImage(ctx, m.Location(), ptr.Deref(m.MachinePool.Spec.Template.Spec.Version, ""))
defaultImage, err = svc.GetDefaultLinuxImage(ctx, m.Location(), ptr.Deref(m.MachinePool.Spec.Template.Spec.Version, ""))
}

if err != nil {
Expand Down
12 changes: 4 additions & 8 deletions azure/scope/machinepool_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -425,14 +425,10 @@ func TestMachinePoolScope_GetVMImage(t *testing.T) {
Verify: func(g *WithT, amp *infrav1exp.AzureMachinePool, vmImage *infrav1.Image, err error) {
g.Expect(err).NotTo(HaveOccurred())
image := &infrav1.Image{
Marketplace: &infrav1.AzureMarketplaceImage{
ImagePlan: infrav1.ImagePlan{
Publisher: "cncf-upstream",
Offer: "capi",
SKU: "k8s-1dot19dot11-ubuntu-1804",
},
Version: "latest",
ThirdPartyImage: false,
ComputeGallery: &infrav1.AzureComputeGalleryImage{
Gallery: "ClusterAPI-f72ceb4f-5159-4c26-a0fe-2ea738f0d019",
Name: "capi-ubun2-2404",
Version: "1.19.11",
},
}
g.Expect(vmImage).To(Equal(image))
Expand Down
Loading
Loading