Skip to content
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
4 changes: 2 additions & 2 deletions aks-node-controller/app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,11 @@ func TestApp_Run(t *testing.T) {
t.Run("download-hotfix returns success when VHD version already matches target", func(t *testing.T) {
tt := NewTestApp(t, TestAppConfig{})
origVersion := Version
Version = "202603.30.0-hotfix1"
Version = "202604.01.1"
defer func() { Version = origVersion }()

configPath := filepath.Join(t.TempDir(), "hotfix-config.json")
require.NoError(t, os.WriteFile(configPath, []byte(`{"version": "202603.30.0-hotfix1"}`), 0o644))
require.NoError(t, os.WriteFile(configPath, []byte(`{"version": "202604.01.1"}`), 0o644))
tt.App.hotfixVersionPath = configPath

exitCode := tt.App.Run(context.Background(), []string{"aks-node-controller", "download-hotfix"})
Expand Down
38 changes: 36 additions & 2 deletions aks-node-controller/hotfix.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import (
"path/filepath"
"strings"
"time"

"github.com/Masterminds/semver/v3"
)

const (
Expand Down Expand Up @@ -44,8 +46,17 @@ func (a *App) downloadHotfix(ctx context.Context) error {
return nil
}

if Version == hotfixVersion {
slog.Info("ANC already at hotfix version, skipping download", "version", Version)
// Patch-only matching: only upgrade if same YYYYMM.DD base and hotfix has
// a strictly higher PATCH. Parse errors (e.g. "dev" builds) result in skip.
shouldUpgrade, err := shouldUpgradeToHotfix(Version, hotfixVersion)
if err != nil {
slog.Warn("failed to compare versions, skipping hotfix download",
"current", Version, "hotfix", hotfixVersion, "error", err)
return nil
}
if !shouldUpgrade {
slog.Info("ANC version not targeted by hotfix, skipping download",
"current", Version, "hotfix", hotfixVersion)
return nil
}

Expand Down Expand Up @@ -278,3 +289,26 @@ func copyBinaryAlongside(src, dst, refPath string) error {
slog.Info("installed hotfix binary alongside VHD binary", "src", src, "hotfixPath", dst)
return nil
}

// shouldUpgradeToHotfix returns true when the current ANC version should be upgraded
// to the hotfix version. This is true only when both versions share the same YYYYMM.DD
// base and the hotfix has a strictly higher PATCH number (patch-only matching).
//
// ANC versions use the format YYYYMM.DD.PATCH which is valid semver (Major.Minor.Patch).
//
// This ensures the hotfix only targets the specific VHD it was built for:
// - Older VHDs (different base) are skipped — remediated via VHD republish
// - Newer VHDs (different base) are skipped — fix is already baked in
// - Same version is skipped — already at hotfix
// - Unparseable versions (e.g. "dev") return an error — caller should skip
func shouldUpgradeToHotfix(current, hotfix string) (bool, error) {
cv, err := semver.NewVersion(strings.TrimSpace(current))
if err != nil {
return false, fmt.Errorf("parsing current version %q: %w", current, err)
}
Comment thread
Devinwong marked this conversation as resolved.
hv, err := semver.NewVersion(strings.TrimSpace(hotfix))
if err != nil {
return false, fmt.Errorf("parsing hotfix version %q: %w", hotfix, err)
}
return cv.Major() == hv.Major() && cv.Minor() == hv.Minor() && hv.Patch() > cv.Patch(), nil
Comment thread
Devinwong marked this conversation as resolved.
}
122 changes: 120 additions & 2 deletions aks-node-controller/hotfix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,18 +107,89 @@ func TestDownloadHotfix_NoHotfixFile(t *testing.T) {

func TestDownloadHotfix_VersionMatch(t *testing.T) {
origVersion := Version
Version = "202603.30.0-hotfix1"
Version = "202604.01.1"
defer func() { Version = origVersion }()

dir := t.TempDir()
path := filepath.Join(dir, "hotfix-config.json")
require.NoError(t, os.WriteFile(path, []byte(`{"version": "202603.30.0-hotfix1"}`), 0o644))
require.NoError(t, os.WriteFile(path, []byte(`{"version": "202604.01.1"}`), 0o644))

tt := NewTestApp(t, TestAppConfig{})
tt.App.hotfixVersionPath = path
require.NoError(t, tt.App.downloadHotfix(context.Background()))
}

func TestDownloadHotfix_DifferentBaseSkips(t *testing.T) {
origVersion := Version
Version = "202605.01.0"
defer func() { Version = origVersion }()

dir := t.TempDir()
path := filepath.Join(dir, "hotfix-config.json")
require.NoError(t, os.WriteFile(path, []byte(`{"version": "202604.01.1"}`), 0o644))

installCalled := false
tt := NewTestApp(t, TestAppConfig{
RunFunc: func(cmd *exec.Cmd) error {
installCalled = true
return nil
},
})
tt.App.hotfixVersionPath = path
require.NoError(t, tt.App.downloadHotfix(context.Background()))
assert.False(t, installCalled, "should skip when version base doesn't match")
}

func TestDownloadHotfix_DevVersionSkips(t *testing.T) {
origVersion := Version
Version = "dev"
defer func() { Version = origVersion }()

dir := t.TempDir()
path := filepath.Join(dir, "hotfix-config.json")
require.NoError(t, os.WriteFile(path, []byte(`{"version": "202604.01.1"}`), 0o644))

installCalled := false
tt := NewTestApp(t, TestAppConfig{
RunFunc: func(cmd *exec.Cmd) error {
installCalled = true
return nil
},
})
tt.App.hotfixVersionPath = path
require.NoError(t, tt.App.downloadHotfix(context.Background()))
assert.False(t, installCalled, "should skip when Version is 'dev' (parse error)")
}

func TestDownloadHotfix_MatchingBaseUpgrades(t *testing.T) {
origVersion := Version
Version = "202604.01.0"
defer func() { Version = origVersion }()

dir := t.TempDir()
path := filepath.Join(dir, "hotfix-config.json")
require.NoError(t, os.WriteFile(path, []byte(`{"version": "202604.01.1"}`), 0o644))

aptDir := filepath.Join(dir, "sources.list.d")
require.NoError(t, os.MkdirAll(aptDir, 0755))
require.NoError(t, os.WriteFile(filepath.Join(aptDir, "microsoft-prod.list"), []byte("deb ..."), 0o644))

installCalled := false
tt := NewTestApp(t, TestAppConfig{
RunFunc: func(cmd *exec.Cmd) error {
installCalled = true
return nil
},
})
tt.App.hotfixVersionPath = path
tt.App.aptSourcesDir = aptDir
// Will fail at copyBinaryAlongside since pkgBinaryPath doesn't exist in test,
// but install should have been called.
err := tt.App.downloadHotfix(context.Background())
require.Error(t, err)
assert.True(t, installCalled, "should proceed when base matches and hotfix patch is higher")
}

func TestDownloadHotfix_UnreadableFile(t *testing.T) {
dir := t.TempDir()
path := filepath.Join(dir, "hotfix-config.json")
Expand Down Expand Up @@ -254,3 +325,50 @@ func TestCopyBinaryAlongside(t *testing.T) {
}
})
}

func TestShouldUpgradeToHotfix(t *testing.T) {
tests := []struct {
name string
current string
hotfix string
want bool
wantErr bool
}{
// Positive: same base, hotfix has higher patch
{"base .0 → hotfix .1", "202604.01.0", "202604.01.1", true, false},
{"base .0 → hotfix .2", "202604.01.0", "202604.01.2", true, false},
{"hotfix .1 → hotfix .2", "202604.01.1", "202604.01.2", true, false},

// Negative: same version
{"same version .0", "202604.01.0", "202604.01.0", false, false},
{"same version .1", "202604.01.1", "202604.01.1", false, false},

// Negative: different base (different YYYYMM)
{"different month", "202603.15.0", "202604.01.1", false, false},
{"newer month", "202605.01.0", "202604.01.1", false, false},

// Negative: different base (different DD)
{"different day", "202604.15.0", "202604.01.1", false, false},

// Negative: current patch higher than hotfix
{"current patch higher", "202604.01.2", "202604.01.1", false, false},

// Error cases
{"dev current", "dev", "202604.01.1", false, true},
{"dev hotfix", "202604.01.0", "dev", false, true},
{"both dev", "dev", "dev", false, true},
{"empty current", "", "202604.01.1", false, true},
{"empty hotfix", "202604.01.0", "", false, true},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
got, err := shouldUpgradeToHotfix(tc.current, tc.hotfix)
if tc.wantErr {
assert.Error(t, err)
} else {
require.NoError(t, err)
assert.Equal(t, tc.want, got, "current=%s hotfix=%s", tc.current, tc.hotfix)
}
})
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,11 @@ log() {
${__SOURCED__:+return}

if [ -f "$HOTFIX_JSON" ]; then
log "Downloading ANC hotfix from ${HOTFIX_JSON}"
log "Found ANC hotfix config at ${HOTFIX_JSON}; running download-hotfix"
if "$BIN_PATH" download-hotfix; then
log "Finished downloading ANC hotfix from ${HOTFIX_JSON}"
log "ANC download-hotfix completed; binary selection follows"
else
log "Failed to download ANC hotfix from ${HOTFIX_JSON}; falling back to staged binaries"
log "ANC download-hotfix failed; binary selection follows"
fi
fi

Expand Down
Loading