-
Notifications
You must be signed in to change notification settings - Fork 251
feat(e2e): dynamically fetch VM extension version with caching, timeouts, and fallback #8064
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
Changes from all commits
943db32
4c25435
a34b749
954bca5
6e8e35b
efdd2c7
4e1e38d
f407bc9
4599da9
7200bfd
ba66c73
7eb2c58
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,123 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| package e2e | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "context" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "fmt" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "sync/atomic" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "testing" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "time" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "github.com/stretchr/testify/assert" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "github.com/stretchr/testify/require" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func Test_cachedFunc_returns_consistent_results(t *testing.T) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| var callCount atomic.Int32 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fn := cachedFunc(func(ctx context.Context, key string) (string, error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| callCount.Add(1) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return "result-" + key, nil | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ctx := context.Background() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| first, err := fn(ctx, "a") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| second, err := fn(ctx, "a") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert.Equal(t, first, second, "cached function should return the same result on repeated calls") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert.Equal(t, int32(1), callCount.Load(), "underlying function should only be called once for the same key") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func Test_cachedFunc_warm_call_is_faster_than_cold(t *testing.T) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fn := cachedFunc(func(ctx context.Context, key string) (string, error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // simulate a slow operation like a network call | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| time.Sleep(10 * time.Millisecond) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return "result", nil | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ctx := context.Background() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| start := time.Now() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _, err := fn(ctx, "key") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| coldDuration := time.Since(start) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| start = time.Now() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _, err = fn(ctx, "key") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| warmDuration := time.Since(start) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| require.NoError(t, err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert.Less(t, warmDuration, coldDuration, "warm (cached) call should be faster than cold call") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+33
to
+51
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fn := cachedFunc(func(ctx context.Context, key string) (string, error) { | |
| // simulate a slow operation like a network call | |
| time.Sleep(10 * time.Millisecond) | |
| return "result", nil | |
| }) | |
| ctx := context.Background() | |
| start := time.Now() | |
| _, err := fn(ctx, "key") | |
| coldDuration := time.Since(start) | |
| require.NoError(t, err) | |
| start = time.Now() | |
| _, err = fn(ctx, "key") | |
| warmDuration := time.Since(start) | |
| require.NoError(t, err) | |
| assert.Less(t, warmDuration, coldDuration, "warm (cached) call should be faster than cold call") | |
| var callCount atomic.Int32 | |
| fn := cachedFunc(func(ctx context.Context, key string) (string, error) { | |
| // simulate a slow operation like a network call | |
| callCount.Add(1) | |
| time.Sleep(10 * time.Millisecond) | |
| return "result", nil | |
| }) | |
| ctx := context.Background() | |
| _, err := fn(ctx, "key") | |
| require.NoError(t, err) | |
| _, err = fn(ctx, "key") | |
| require.NoError(t, err) | |
| assert.Equal(t, int32(1), callCount.Load(), "warm (cached) call should not invoke the slow underlying function again for the same key") |
Copilot
AI
Mar 26, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test_cachedFunc_warm_call_is_faster_than_cold compares wall-clock durations to assert caching, which can be flaky due to scheduler noise (warm call can occasionally be slower). A more deterministic assertion is to track underlying function invocations (e.g., via an atomic counter) and assert it’s called exactly once per key, optionally keeping the sleep to make the cold path slower but not asserting on timing.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,15 @@ | ||
| package config | ||
|
|
||
| import ( | ||
| "cmp" | ||
| "context" | ||
| "crypto/tls" | ||
| "errors" | ||
| "fmt" | ||
| "net" | ||
| "net/http" | ||
| "os" | ||
| "sort" | ||
| "slices" | ||
| "strconv" | ||
| "strings" | ||
| "time" | ||
|
|
@@ -745,75 +746,99 @@ func (a *AzureClient) DeleteSnapshot(ctx context.Context, resourceGroupName, sna | |
| return nil | ||
| } | ||
|
|
||
| // vmExtensionImageVersionLister abstracts the ListVersions method of the VM extension images client for testability. | ||
| type vmExtensionImageVersionLister interface { | ||
| ListVersions(ctx context.Context, location string, publisherName string, typeParam string, | ||
| options *armcompute.VirtualMachineExtensionImagesClientListVersionsOptions, | ||
| ) (armcompute.VirtualMachineExtensionImagesClientListVersionsResponse, error) | ||
| } | ||
|
|
||
| // GetLatestVMExtensionImageVersion lists VM extension images for a given extension name and returns the latest version. | ||
| // This is equivalent to: az vm extension image list -n Compute.AKS.Linux.AKSNode --latest | ||
| func (a *AzureClient) GetLatestVMExtensionImageVersion(ctx context.Context, location, extType, extPublisher string) (string, error) { | ||
| return getLatestVMExtensionImageVersion(ctx, a.VMExtensionImages, location, extType, extPublisher) | ||
| } | ||
|
|
||
| // getLatestVMExtensionImageVersion lists VM extension images using the provided lister and returns the latest version. | ||
| func getLatestVMExtensionImageVersion(ctx context.Context, lister vmExtensionImageVersionLister, location, extType, extPublisher string) (string, error) { | ||
| // List extension versions | ||
| resp, err := a.VMExtensionImages.ListVersions(ctx, location, extPublisher, extType, &armcompute.VirtualMachineExtensionImagesClientListVersionsOptions{}) | ||
| resp, err := lister.ListVersions(ctx, location, extPublisher, extType, &armcompute.VirtualMachineExtensionImagesClientListVersionsOptions{}) | ||
| if err != nil { | ||
| return "", fmt.Errorf("listing extension versions: %w", err) | ||
| } | ||
|
|
||
| if len(resp.VirtualMachineExtensionImageArray) == 0 { | ||
| return "", fmt.Errorf("no extension versions found") | ||
| } | ||
|
|
||
| version := make([]VMExtenstionVersion, len(resp.VirtualMachineExtensionImageArray)) | ||
| versions := make([]vmExtensionVersion, len(resp.VirtualMachineExtensionImageArray)) | ||
| for i, ext := range resp.VirtualMachineExtensionImageArray { | ||
| version[i] = parseVersion(ext) | ||
| versions[i] = parseVersion(ctx, ext) | ||
| } | ||
|
|
||
| sort.Slice(version, func(i, j int) bool { | ||
| return version[i].Less(version[j]) | ||
| latest := slices.MaxFunc(versions, func(a, b vmExtensionVersion) int { | ||
| return a.cmp(b) | ||
| }) | ||
|
|
||
| return *version[len(version)-1].Original.Name, nil | ||
| if latest.original.Name == nil { | ||
| return "", fmt.Errorf("latest extension version has nil name") | ||
|
Comment on lines
+778
to
+782
|
||
| } | ||
| return *latest.original.Name, nil | ||
|
surajssd marked this conversation as resolved.
|
||
| } | ||
|
|
||
| // VMExtenstionVersion represents a parsed version of a VM extension image. | ||
| type VMExtenstionVersion struct { | ||
| Original *armcompute.VirtualMachineExtensionImage | ||
| Major int | ||
| Minor int | ||
| Patch int | ||
| // vmExtensionVersion represents a parsed version of a VM extension image. | ||
| type vmExtensionVersion struct { | ||
| original *armcompute.VirtualMachineExtensionImage | ||
| major int | ||
| minor int | ||
| patch int | ||
| } | ||
|
|
||
| // parseVersion parses the version from a VM extension image name, which can be in the format 1.151, 1.0.1, etc. | ||
| // You can find all the versions of a specific VM extension by running: | ||
| // az vm extension image list -n Compute.AKS.Linux.AKSNode | ||
| func parseVersion(v *armcompute.VirtualMachineExtensionImage) VMExtenstionVersion { | ||
| func parseVersion(ctx context.Context, v *armcompute.VirtualMachineExtensionImage) vmExtensionVersion { | ||
| version := vmExtensionVersion{original: v} | ||
| if v.Name == nil { | ||
| toolkit.Logf(ctx, "warning: VM extension image has nil name, skipping version parse") | ||
| return version | ||
| } | ||
|
|
||
| // Split by dots | ||
| parts := strings.Split(*v.Name, ".") | ||
|
|
||
| version := VMExtenstionVersion{Original: v} | ||
|
|
||
| if len(parts) >= 1 { | ||
| if major, err := strconv.Atoi(parts[0]); err == nil { | ||
| version.Major = major | ||
| version.major = major | ||
| } else { | ||
| toolkit.Logf(ctx, "warning: failed to parse major version from %q: %v", *v.Name, err) | ||
| } | ||
| } | ||
| if len(parts) >= 2 { | ||
| if minor, err := strconv.Atoi(parts[1]); err == nil { | ||
| version.Minor = minor | ||
| version.minor = minor | ||
| } else { | ||
| toolkit.Logf(ctx, "warning: failed to parse minor version from %q: %v", *v.Name, err) | ||
| } | ||
| } | ||
| if len(parts) >= 3 { | ||
| if patch, err := strconv.Atoi(parts[2]); err == nil { | ||
| version.Patch = patch | ||
| version.patch = patch | ||
| } else { | ||
| toolkit.Logf(ctx, "warning: failed to parse patch version from %q: %v", *v.Name, err) | ||
| } | ||
|
surajssd marked this conversation as resolved.
|
||
| } | ||
|
|
||
| return version | ||
| } | ||
|
|
||
| func (v VMExtenstionVersion) Less(other VMExtenstionVersion) bool { | ||
| if v.Major != other.Major { | ||
| return v.Major < other.Major | ||
| // cmp compares two versions, returning -1, 0, or 1. | ||
| func (v vmExtensionVersion) cmp(other vmExtensionVersion) int { | ||
| if c := cmp.Compare(v.major, other.major); c != 0 { | ||
| return c | ||
| } | ||
| if v.Minor != other.Minor { | ||
| return v.Minor < other.Minor | ||
| if c := cmp.Compare(v.minor, other.minor); c != 0 { | ||
| return c | ||
| } | ||
| return v.Patch < other.Patch | ||
| return cmp.Compare(v.patch, other.patch) | ||
| } | ||
|
|
||
| // getResourceSKU queries the Azure Resource SKUs API to find the SKU for the given VM size and location. | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.