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

Add workload cluster upgrades spec #1767

Merged
Merged
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
2 changes: 1 addition & 1 deletion test/e2e/aks.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ func GetWorkingAKSKubernetesVersion(ctx context.Context, subscriptionID, locatio
var latestStableVersionDesired bool
// We're not doing much input validation here,
// we assume that if the prefix is 'stable-' that the remainder of the string is in the format <Major>.<Minor>
if version[:7] == "stable-" && validateStableReleaseString(version) {
if isStableVersion, _ := validateStableReleaseString(version); isStableVersion {
latestStableVersionDesired = true
// Form a fully valid semver version @ the initial patch release (".0")
version = fmt.Sprintf("%s.0", version[7:])
Expand Down
68 changes: 37 additions & 31 deletions test/e2e/capi_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,13 @@ import (
"context"
"encoding/base64"
"fmt"
"os"

"github.com/blang/semver"
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"os"
e2e_namespace "sigs.k8s.io/cluster-api-provider-azure/test/e2e/kubernetes/namespace"
clusterctl "sigs.k8s.io/cluster-api/cmd/clusterctl/api/v1alpha3"
capi_e2e "sigs.k8s.io/cluster-api/test/e2e"
Expand All @@ -45,14 +47,22 @@ var _ = Describe("Running the Cluster API E2E tests", func() {
BeforeEach(func() {
Expect(e2eConfig.Variables).To(HaveKey(capi_e2e.CNIPath))
rgName := fmt.Sprintf("capz-e2e-%s", util.RandomString(6))
Expect(os.Setenv(AzureResourceGroup, rgName)).NotTo(HaveOccurred())
Expect(os.Setenv(AzureVNetName, fmt.Sprintf("%s-vnet", rgName))).NotTo(HaveOccurred())
Expect(os.Setenv(AzureResourceGroup, rgName)).To(Succeed())
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for changing these to To(Succeed()). So much more readable!

Expect(os.Setenv(AzureVNetName, fmt.Sprintf("%s-vnet", rgName))).To(Succeed())

Expect(e2eConfig.Variables).To(HaveKey(capi_e2e.KubernetesVersionUpgradeFrom))
v, err := semver.ParseTolerant(e2eConfig.GetVariable(capi_e2e.KubernetesVersionUpgradeFrom))
Expect(err).NotTo(HaveOccurred())
// Opt into Windows for versions greater than or equal to 1.22
if v.GTE(semver.MustParse("1.22.0")) {
Expect(os.Setenv("WINDOWS_WORKER_MACHINE_COUNT", "2")).To(Succeed())
Expect(os.Setenv("K8S_FEATURE_GATES", "WindowsHostProcessContainers=true")).To(Succeed())
}

clientset := bootstrapClusterProxy.GetClientSet()
Expect(clientset).NotTo(BeNil())
ns := fmt.Sprintf("capz-e2e-identity-%s", util.RandomString(6))

var err error
identityNamespace, err = e2e_namespace.Create(ctx, clientset, ns, map[string]string{})
Expect(err).ToNot(HaveOccurred())

Expand All @@ -72,23 +82,19 @@ var _ = Describe("Running the Cluster API E2E tests", func() {
Expect(err).ToNot(HaveOccurred())

identityName := e2eConfig.GetVariable(ClusterIdentityName)
Expect(os.Setenv(ClusterIdentityName, identityName)).NotTo(HaveOccurred())
Expect(os.Setenv(ClusterIdentitySecretName, IdentitySecretName)).NotTo(HaveOccurred())
Expect(os.Setenv(ClusterIdentitySecretNamespace, identityNamespace.Name)).NotTo(HaveOccurred())

// Opt into using windows with prow template
Expect(os.Setenv("WINDOWS_WORKER_MACHINE_COUNT", "2")).To(Succeed())
Expect(os.Setenv("K8S_FEATURE_GATES", "WindowsHostProcessContainers=true")).To(Succeed())
Expect(os.Setenv(ClusterIdentityName, identityName)).To(Succeed())
Expect(os.Setenv(ClusterIdentitySecretName, IdentitySecretName)).To(Succeed())
Expect(os.Setenv(ClusterIdentitySecretNamespace, identityNamespace.Name)).To(Succeed())
})

AfterEach(func() {
redactLogs()

Expect(os.Unsetenv(AzureResourceGroup)).NotTo(HaveOccurred())
Expect(os.Unsetenv(AzureVNetName)).NotTo(HaveOccurred())
Expect(os.Unsetenv(ClusterIdentityName)).NotTo(HaveOccurred())
Expect(os.Unsetenv(ClusterIdentitySecretName)).NotTo(HaveOccurred())
Expect(os.Unsetenv(ClusterIdentitySecretNamespace)).NotTo(HaveOccurred())
Expect(os.Unsetenv(AzureResourceGroup)).To(Succeed())
Expect(os.Unsetenv(AzureVNetName)).To(Succeed())
Expect(os.Unsetenv(ClusterIdentityName)).To(Succeed())
Expect(os.Unsetenv(ClusterIdentitySecretName)).To(Succeed())
Expect(os.Unsetenv(ClusterIdentitySecretNamespace)).To(Succeed())
})

Context("Running the quick-start spec", func() {
Expand All @@ -103,20 +109,7 @@ var _ = Describe("Running the Cluster API E2E tests", func() {
})
})

Context("Running the KCP upgrade spec in a single control plane cluster", func() {
capi_e2e.KCPUpgradeSpec(context.TODO(), func() capi_e2e.KCPUpgradeSpecInput {
return capi_e2e.KCPUpgradeSpecInput{
E2EConfig: e2eConfig,
ClusterctlConfigPath: clusterctlConfigPath,
BootstrapClusterProxy: bootstrapClusterProxy,
ArtifactFolder: artifactFolder,
ControlPlaneMachineCount: 1,
SkipCleanup: skipCleanup,
}
})
})

Context("Running the KCP upgrade spec in a HA cluster", func() {
Context("Running the KCP upgrade spec in a HA cluster [K8s-Upgrade]", func() {
capi_e2e.KCPUpgradeSpec(context.TODO(), func() capi_e2e.KCPUpgradeSpecInput {
return capi_e2e.KCPUpgradeSpecInput{
E2EConfig: e2eConfig,
Expand All @@ -129,7 +122,7 @@ var _ = Describe("Running the Cluster API E2E tests", func() {
})
})

Context("Running the KCP upgrade spec in a HA cluster using scale in rollout", func() {
Context("Running the KCP upgrade spec in a HA cluster using scale in rollout [K8s-Upgrade]", func() {
capi_e2e.KCPUpgradeSpec(context.TODO(), func() capi_e2e.KCPUpgradeSpecInput {
return capi_e2e.KCPUpgradeSpecInput{
E2EConfig: e2eConfig,
Expand Down Expand Up @@ -258,4 +251,17 @@ var _ = Describe("Running the Cluster API E2E tests", func() {
})
})
}

Context("Running the workload cluster upgrade spec [K8s-Upgrade]", func() {
capi_e2e.ClusterUpgradeConformanceSpec(ctx, func() capi_e2e.ClusterUpgradeConformanceSpecInput {
return capi_e2e.ClusterUpgradeConformanceSpecInput{
E2EConfig: e2eConfig,
ClusterctlConfigPath: clusterctlConfigPath,
BootstrapClusterProxy: bootstrapClusterProxy,
ArtifactFolder: artifactFolder,
SkipCleanup: skipCleanup,
SkipConformanceTests: true,
}
})
})
})
2 changes: 2 additions & 0 deletions test/e2e/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ const (
Timestamp = "TIMESTAMP"
AKSKubernetesVersion = "AKS_KUBERNETES_VERSION"
ManagedClustersResourceType = "managedClusters"
capiImagePublisher = "cncf-upstream"
capiOfferName = "capi"
)

func Byf(format string, a ...interface{}) {
Expand Down
18 changes: 7 additions & 11 deletions test/e2e/config/azure-dev.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -92,19 +92,15 @@ providers:
- sourcePath: "../data/infrastructure-azure/v1beta1/cluster-template.yaml"
targetName: "cluster-template-management.yaml"
- sourcePath: "../data/infrastructure-azure/v1beta1/cluster-template-kcp-adoption.yaml"
targetName: "cluster-template-kcp-adoption.yaml"
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm assuming that targetName can be omitted if it agrees with the last element of sourcePath.

- sourcePath: "${PWD}/templates/test/ci/cluster-template-prow-ipv6.yaml"
targetName: "cluster-template-ipv6.yaml"
- sourcePath: "../data/infrastructure-azure/v1beta1/cluster-template-md-remediation.yaml"
targetName: "cluster-template-md-remediation.yaml"
- sourcePath: "../data/infrastructure-azure/v1beta1/cluster-template-kcp-remediation.yaml"
targetName: "cluster-template-kcp-remediation.yaml"
- sourcePath: "../data/infrastructure-azure/v1beta1/cluster-template-kcp-scale-in.yaml"
targetName: "cluster-template-kcp-scale-in.yaml"
- sourcePath: "../data/infrastructure-azure/v1beta1/cluster-template-node-drain.yaml"
targetName: "cluster-template-node-drain.yaml"
- sourcePath: "../data/infrastructure-azure/v1beta1/cluster-template-upgrades.yaml"
- sourcePath: "${PWD}/templates/test/ci/cluster-template-prow-machine-pool.yaml"
targetName: "cluster-template-machine-pool.yaml"
- sourcePath: "${PWD}/templates/test/ci/cluster-template-prow-ipv6.yaml"
targetName: "cluster-template-ipv6.yaml"
Copy link
Contributor

Choose a reason for hiding this comment

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

...but targetName is needed here because...sourcePath isn't relative? Or something? I'm mildly confused by this cleanup.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah basically targetName is only needed if we want to rename the file, otherwise it just uses the filename. In this case we want to remove the "prow" part from the file name so it can be used as the "ipv6" flavor.

- sourcePath: "${PWD}/templates/test/ci/cluster-template-prow-nvidia-gpu.yaml"
targetName: "cluster-template-nvidia-gpu.yaml"
- sourcePath: "${PWD}/templates/test/ci/cluster-template-prow-private.yaml"
Expand Down Expand Up @@ -138,18 +134,17 @@ providers:
new: "--v=2"

variables:
KUBERNETES_VERSION: "${KUBERNETES_VERSION:-v1.22.1}"
AKS_KUBERNETES_VERSION: "${KUBERNETES_VERSION:-stable-1.19}"
KUBERNETES_VERSION: "${KUBERNETES_VERSION:=stable-1.22}"
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

ETCD_VERSION_UPGRADE_TO: "3.5.0-0"
COREDNS_VERSION_UPGRADE_TO: "v1.8.4"
KUBERNETES_VERSION_UPGRADE_TO: "${KUBERNETES_VERSION_UPGRADE_TO:-v1.22.2}"
KUBERNETES_VERSION_UPGRADE_FROM: "${KUBERNETES_VERSION_UPGRADE_FROM:-v1.22.1}"
KUBERNETES_VERSION_UPGRADE_TO: "${KUBERNETES_VERSION_UPGRADE_TO:=stable-1.22}"
KUBERNETES_VERSION_UPGRADE_FROM: "${KUBERNETES_VERSION_UPGRADE_FROM:-v1.22.1}" # this needs to be 1.22+ to support mixed windows/linux clusters until we update CAPI to v1.0.1.
CNI: "${PWD}/templates/addons/calico.yaml"
REDACT_LOG_SCRIPT: "${PWD}/hack/log/redact.sh"
EXP_AKS: "true"
EXP_MACHINE_POOL: "true"
EXP_CLUSTER_RESOURCE_SET: "true"
CONFORMANCE_CI_ARTIFACTS_KUBERNETES_VERSION: "v1.22.1"
CONFORMANCE_WORKER_MACHINE_COUNT: "2"
CONFORMANCE_CONTROL_PLANE_MACHINE_COUNT: "${CONFORMANCE_CONTROL_PLANE_MACHINE_COUNT:-1}"
CONFORMANCE_IMAGE: "${CONFORMANCE_IMAGE:-}"
Expand All @@ -164,6 +159,7 @@ variables:
INIT_WITH_BINARY: "https://github.com/kubernetes-sigs/cluster-api/releases/download/v0.3.23/clusterctl-{OS}-{ARCH}"
INIT_WITH_PROVIDERS_CONTRACT: "v1alpha3"
INIT_WITH_KUBERNETES_VERSION: "v1.21.2"
KUBETEST_CONFIGURATION: "./data/kubetest/conformance.yaml"

intervals:
default/wait-controllers: ["3m", "10s"]
Expand Down
102 changes: 100 additions & 2 deletions test/e2e/e2e_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,17 @@ import (
"path"
"path/filepath"
"regexp"
"strconv"
"strings"
"testing"
"time"

aadpodv1 "github.com/Azure/aad-pod-identity/pkg/apis/aadpodidentity/v1"
"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2021-07-01/compute"
"github.com/Azure/azure-sdk-for-go/services/preview/monitor/mgmt/2019-06-01/insights"
"github.com/Azure/go-autorest/autorest/azure/auth"
"github.com/Azure/go-autorest/autorest/to"
"github.com/blang/semver"
. "github.com/onsi/ginkgo"
"github.com/onsi/ginkgo/config"
"github.com/onsi/ginkgo/reporters"
Expand Down Expand Up @@ -344,6 +347,8 @@ func loadE2EConfig(configPath string) *clusterctl.E2EConfig {
config := clusterctl.LoadE2EConfig(context.TODO(), clusterctl.LoadE2EConfigInput{ConfigPath: configPath})
Expect(config).ToNot(BeNil(), "Failed to load E2E config from %s", configPath)

resolveKubernetesVersions(config)

return config
}

Expand Down Expand Up @@ -403,10 +408,103 @@ func tearDown(bootstrapClusterProvider bootstrap.ClusterProvider, bootstrapClust
}
}

// resolveKubernetesVersions looks at Kubernetes versions set as variables in the e2e config and sets them to a valid k8s version
// that has an existing capi offer image available. For example, if the version is "stable-1.22", the function will set it to the latest 1.22 version that has a published reference image.
func resolveKubernetesVersions(config *clusterctl.E2EConfig) {
skus := getAllCAPIImageSkus(context.TODO(), os.Getenv(AzureLocation))
versions := parseImageSkuNames(skus)

if config.HasVariable(capi_e2e.KubernetesVersion) {
resolveKubernetesVersion(config, versions, capi_e2e.KubernetesVersion)
}
if config.HasVariable(capi_e2e.KubernetesVersionUpgradeFrom) {
resolveKubernetesVersion(config, versions, capi_e2e.KubernetesVersionUpgradeFrom)
}
if config.HasVariable(capi_e2e.KubernetesVersionUpgradeTo) {
resolveKubernetesVersion(config, versions, capi_e2e.KubernetesVersionUpgradeTo)
}
}

func resolveKubernetesVersion(config *clusterctl.E2EConfig, versions semver.Versions, varName string) {
v := getLatestSkuForMinor(config.GetVariable(varName), versions)
if _, ok := os.LookupEnv(varName); ok {
Expect(os.Setenv(varName, v)).To(Succeed())
Copy link
Contributor

Choose a reason for hiding this comment

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

do we want to mutate env[varName] here? I understand why we are doing it but wondering if it'll have any undue side effects.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At first I didn't have this (only the config.Variables part below), but it didn't work. Unfortunately that's the only way to modify the config since the capi framework gives precedence to env variables if they are set: https://github.com/kubernetes-sigs/cluster-api/blob/main/test/framework/clusterctl/e2e_config.go#L553

FWIW, this is similar to what CAPI does for CAPD e2e tests https://github.com/kubernetes-sigs/cluster-api/blob/main/scripts/ci-e2e-lib.sh#L56

Copy link
Contributor

Choose a reason for hiding this comment

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

oh it's a capi constraint. Thanks for the explanation!

}
config.Variables[varName] = v
}

// getAllCAPIImageSkus returns all skus for the capi offer under the "cncf-upstream" publisher.
func getAllCAPIImageSkus(ctx context.Context, location string) []string {
settings, err := auth.GetSettingsFromEnvironment()
Expect(err).NotTo(HaveOccurred())
subscriptionID := settings.GetSubscriptionID()
authorizer, err := settings.GetAuthorizer()
Expect(err).NotTo(HaveOccurred())
imagesClient := compute.NewVirtualMachineImagesClient(subscriptionID)
imagesClient.Authorizer = authorizer

Byf("Finding image skus for offer %s/%s in %s", capiImagePublisher, capiOfferName, location)

res, err := imagesClient.ListSkus(ctx, location, capiImagePublisher, capiOfferName)
Expect(err).NotTo(HaveOccurred())

var skus []string
if res.Value != nil {
skus = make([]string, len(*res.Value))
for i, sku := range *res.Value {
// we have to do this to make sure the SKU has existing images
// see https://github.com/Azure/azure-cli/issues/20115.
res, err := imagesClient.List(ctx, location, capiImagePublisher, capiOfferName, *sku.Name, "", nil, "")
Expect(err).NotTo(HaveOccurred())
if res.Value != nil && len(*res.Value) > 0 {
skus[i] = *sku.Name
}
}
}
return skus
}

// parseImageSkuNames parses SKU names in format "k8s-1dot17dot2-ubuntu-1804" to extract the Kubernetes version.
// it returns a sorted list of all k8s versions found.
func parseImageSkuNames(skus []string) semver.Versions {
CecileRobertMichon marked this conversation as resolved.
Show resolved Hide resolved
var capiSku = regexp.MustCompile(`^k8s-(0|[1-9][0-9]*)dot(0|[1-9][0-9]*)dot(0|[1-9][0-9]*)-ubuntu.*$`)
versions := make(semver.Versions, len(skus))
for i, sku := range skus {
match := capiSku.FindStringSubmatch(sku)
if len(match) != 0 {
versions[i] = semver.MustParse(fmt.Sprintf("%s.%s.%s", match[1], match[2], match[3]))
}
}
return versions
}

// getLatestSkuForMinor gets the latest available patch version in the provided list of sku versions that corresponds to the provided k8s version.
func getLatestSkuForMinor(version string, skus semver.Versions) string {
isStable, match := validateStableReleaseString(version)
if isStable {
major, err := strconv.ParseUint(match[1], 10, 64)
Expect(err).NotTo(HaveOccurred())
minor, err := strconv.ParseUint(match[2], 10, 64)
Expect(err).NotTo(HaveOccurred())
semver.Sort(skus)
for i := len(skus) - 1; i >= 0; i-- {
if skus[i].Major == major && skus[i].Minor == minor {
version = "v" + skus[i].String()
break
}
}
} else {
v, err := semver.ParseTolerant(version)
Expect(err).NotTo(HaveOccurred())
Expect(skus).To(ContainElement(v), fmt.Sprintf("Provided Kubernetes version %s does not have a corresponding VM image in the capi offer", version))
}
CecileRobertMichon marked this conversation as resolved.
Show resolved Hide resolved
return version
}

// validateStableReleaseString validates the string format that declares "get be the latest stable release for this <Major>.<Minor>"
// it should be called wherever we process a stable version string expression like "stable-1.22"
func validateStableReleaseString(stableVersion string) bool {
func validateStableReleaseString(stableVersion string) (bool, []string) {
stableReleaseFormat := regexp.MustCompile(`^stable-(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)$`)
matches := stableReleaseFormat.FindStringSubmatch(stableVersion)
return len(matches) > 0
return len(matches) > 0, matches
}