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
72 changes: 72 additions & 0 deletions apis/actions.github.com/v1alpha1/version.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
package v1alpha1

import "strings"

func IsVersionAllowed(resourceVersion, buildVersion string) bool {
if buildVersion == "dev" || resourceVersion == buildVersion || strings.HasPrefix(buildVersion, "canary-") {
return true
}

rv, ok := parseSemver(resourceVersion)
if !ok {
return false
}
bv, ok := parseSemver(buildVersion)
if !ok {
return false
}
Copy link

Copilot AI May 8, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider adding a comment here to clarify that patch version differences are intentionally ignored.

Suggested change
}
}
// Intentionally ignore patch version differences; only compare major and minor versions.

Copilot uses AI. Check for mistakes.
return rv.major == bv.major && rv.minor == bv.minor
}

type semver struct {
major string
minor string
}

func parseSemver(v string) (p semver, ok bool) {
if v == "" {
return
}
p.major, v, ok = parseInt(v)
if !ok {
return p, false
}
if v == "" {
p.minor = "0"
return p, true
}
if v[0] != '.' {
return p, false
}
p.minor, v, ok = parseInt(v[1:])
if !ok {
return p, false
}
if v == "" {
return p, true
}
if v[0] != '.' {
return p, false
}
if _, _, ok = parseInt(v[1:]); !ok {
return p, false
}
return p, true
}

func parseInt(v string) (t, rest string, ok bool) {
if v == "" {
return
}
if v[0] < '0' || '9' < v[0] {
return
}
i := 1
for i < len(v) && '0' <= v[i] && v[i] <= '9' {
i++
}
if v[0] == '0' && i != 1 {
return
}
return v[:i], v[i:], true
}
60 changes: 60 additions & 0 deletions apis/actions.github.com/v1alpha1/version_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
package v1alpha1_test

import (
"testing"

"github.com/actions/actions-runner-controller/apis/actions.github.com/v1alpha1"
"github.com/stretchr/testify/assert"
)

func TestIsVersionAllowed(t *testing.T) {
t.Parallel()
tt := map[string]struct {
resourceVersion string
buildVersion string
want bool
}{
"dev should always be allowed": {
resourceVersion: "0.11.0",
buildVersion: "dev",
Copy link
Member

Choose a reason for hiding this comment

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

We probably need to accept canary-sha here as well:

build-args: VERSION=canary-${{ steps.resolve_parameters.outputs.short_sha }}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, I thought this one was covered by the exact match, but I guess I can add HasPrefix on canary- to allow mismatches in canary deployments

want: true,
},
"resourceVersion is not semver": {
resourceVersion: "dev",
buildVersion: "0.11.0",
want: false,
},
"buildVersion is not semver": {
resourceVersion: "0.11.0",
buildVersion: "NA",
want: false,
},
"major version mismatch": {
resourceVersion: "0.11.0",
buildVersion: "1.11.0",
want: false,
},
"minor version mismatch": {
resourceVersion: "0.11.0",
buildVersion: "0.10.0",
want: false,
},
"patch version mismatch": {
resourceVersion: "0.11.1",
buildVersion: "0.11.0",
want: true,
},
"arbitrary version match": {
resourceVersion: "abc",
buildVersion: "abc",
want: true,
},
}

for name, tc := range tt {
t.Run(name, func(t *testing.T) {
got := v1alpha1.IsVersionAllowed(tc.resourceVersion, tc.buildVersion)
assert.Equal(t, tc.want, got)
})
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ func (r *AutoscalingRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl
return ctrl.Result{}, nil
}

if autoscalingRunnerSet.Labels[LabelKeyKubernetesVersion] != build.Version {
if !v1alpha1.IsVersionAllowed(autoscalingRunnerSet.Labels[LabelKeyKubernetesVersion], build.Version) {
if err := r.Delete(ctx, autoscalingRunnerSet); err != nil {
log.Error(err, "Failed to delete autoscaling runner set on version mismatch",
"buildVersion", build.Version,
Expand Down
Loading