From 6d2a1cdccec9112dddf9ce2be0742bc2bc521157 Mon Sep 17 00:00:00 2001 From: Nikola Jokic Date: Mon, 6 Jul 2026 14:37:20 +0200 Subject: [PATCH] Use metrics to display runner statuses instead of status field for EphemeralRunnerSet and AutoscalingRunnerSet --- .../v1alpha1/autoscalingrunnerset_types.go | 51 +-- .../v1alpha1/ephemeralrunnerset_types.go | 10 +- .../v1alpha1/proxy_config_test.go | 8 +- ...tions.github.com_autoscalinglisteners.yaml | 30 +- ...ions.github.com_autoscalingrunnersets.yaml | 35 +-- .../actions.github.com_ephemeralrunners.yaml | 23 +- ...ctions.github.com_ephemeralrunnersets.yaml | 25 +- ...tions.github.com_autoscalinglisteners.yaml | 30 +- ...ions.github.com_autoscalingrunnersets.yaml | 35 +-- .../actions.github.com_ephemeralrunners.yaml | 23 +- ...ctions.github.com_ephemeralrunnersets.yaml | 25 +- .../templates/_mode_kubernetes.tpl | 5 +- .../templates/manager_role.yaml | 2 - .../templates/manager_role.yaml | 2 - .../tests/template_test.go | 4 +- ...tions.github.com_autoscalinglisteners.yaml | 30 +- ...ions.github.com_autoscalingrunnersets.yaml | 35 +-- .../actions.github.com_ephemeralrunners.yaml | 23 +- ...ctions.github.com_ephemeralrunnersets.yaml | 25 +- config/rbac/role.yaml | 4 +- .../autoscalinglistener_controller.go | 174 +++++------ .../autoscalinglistener_controller_test.go | 10 +- .../autoscalingrunnerset_controller.go | 52 +++- .../autoscalingrunnerset_controller_test.go | 290 +++++++----------- .../ephemeralrunner_controller_test.go | 127 ++++---- .../ephemeralrunnerset_controller_test.go | 42 +-- .../actions.github.com/resourcebuilder.go | 135 +++----- .../resourcebuilder_test.go | 8 +- .../secretresolver/secret_resolver.go | 8 +- controllers/actions.github.com/utils.go | 10 - 30 files changed, 532 insertions(+), 749 deletions(-) diff --git a/apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go b/apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go index f268d874..11c9edd9 100644 --- a/apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go +++ b/apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go @@ -36,30 +36,23 @@ import ( // +kubebuilder:subresource:status // +kubebuilder:printcolumn:JSONPath=".spec.minRunners",name=Minimum Runners,type=integer // +kubebuilder:printcolumn:JSONPath=".spec.maxRunners",name=Maximum Runners,type=integer -// +kubebuilder:printcolumn:JSONPath=".status.currentRunners",name=Current Runners,type=integer // +kubebuilder:printcolumn:JSONPath=".status.phase",name=Phase,type=string -// +kubebuilder:printcolumn:JSONPath=".status.pendingEphemeralRunners",name=Pending Runners,type=integer -// +kubebuilder:printcolumn:JSONPath=".status.runningEphemeralRunners",name=Running Runners,type=integer -// +kubebuilder:printcolumn:JSONPath=".status.failedEphemeralRunners",name=Failed Runners,type=integer // AutoscalingRunnerSet is the Schema for the autoscalingrunnersets API type AutoscalingRunnerSet struct { - metav1.TypeMeta `json:",inline"` - // +optional + metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` - // +optional - Spec AutoscalingRunnerSetSpec `json:"spec,omitempty"` - // +optional + Spec AutoscalingRunnerSetSpec `json:"spec,omitempty"` Status AutoscalingRunnerSetStatus `json:"status,omitempty"` } // AutoscalingRunnerSetSpec defines the desired state of AutoscalingRunnerSet type AutoscalingRunnerSetSpec struct { - // +optional + // Required GitHubConfigUrl string `json:"githubConfigUrl,omitempty"` - // +optional + // Required GitHubConfigSecret string `json:"githubConfigSecret,omitempty"` // +optional @@ -80,7 +73,7 @@ type AutoscalingRunnerSetSpec struct { // +optional VaultConfig *VaultConfig `json:"vaultConfig,omitempty"` - // +optional + // Required Template corev1.PodTemplateSpec `json:"template,omitempty"` // +optional @@ -114,16 +107,16 @@ type AutoscalingRunnerSetSpec struct { EphemeralRunnerConfigSecretMetadata *ResourceMeta `json:"ephemeralRunnerConfigSecretMetadata,omitempty"` // +optional - // +kubebuilder:validation:Minimum=0 + // +kubebuilder:validation:Minimum:=0 MaxRunners *int `json:"maxRunners,omitempty"` // +optional - // +kubebuilder:validation:Minimum=0 + // +kubebuilder:validation:Minimum:=0 MinRunners *int `json:"minRunners,omitempty"` } type TLSConfig struct { - // +required + // Required CertificateFrom *TLSCertificateSource `json:"certificateFrom,omitempty"` } @@ -160,7 +153,7 @@ func (c *TLSConfig) ToCertPool(keyFetcher func(name, key string) ([]byte, error) } type TLSCertificateSource struct { - // +required + // Required ConfigMapKeyRef *corev1.ConfigMapKeySelector `json:"configMapKeyRef,omitempty"` } @@ -181,9 +174,9 @@ func (c *ProxyConfig) ToHTTPProxyConfig(secretFetcher func(string) (*corev1.Secr } if c.HTTP != nil { - u, err := url.Parse(c.HTTP.URL) + u, err := url.Parse(c.HTTP.Url) if err != nil { - return nil, fmt.Errorf("failed to parse proxy http url %q: %w", c.HTTP.URL, err) + return nil, fmt.Errorf("failed to parse proxy http url %q: %w", c.HTTP.Url, err) } if c.HTTP.CredentialSecretRef != "" { @@ -206,9 +199,9 @@ func (c *ProxyConfig) ToHTTPProxyConfig(secretFetcher func(string) (*corev1.Secr } if c.HTTPS != nil { - u, err := url.Parse(c.HTTPS.URL) + u, err := url.Parse(c.HTTPS.Url) if err != nil { - return nil, fmt.Errorf("failed to parse proxy https url %q: %w", c.HTTPS.URL, err) + return nil, fmt.Errorf("failed to parse proxy https url %q: %w", c.HTTPS.Url, err) } if c.HTTPS.CredentialSecretRef != "" { @@ -261,8 +254,8 @@ func (c *ProxyConfig) ProxyFunc(secretFetcher func(string) (*corev1.Secret, erro } type ProxyServerConfig struct { - // +required - URL string `json:"url,omitempty"` + // Required + Url string `json:"url,omitempty"` // +optional CredentialSecretRef string `json:"credentialSecretRef,omitempty"` @@ -330,6 +323,20 @@ const ( AutoscalingRunnerSetPhaseOutdated AutoscalingRunnerSetPhase = "Outdated" ) +func (ars *AutoscalingRunnerSet) Hash() string { + type data struct { + Spec *AutoscalingRunnerSetSpec + Labels map[string]string + } + + d := &data{ + Spec: ars.Spec.DeepCopy(), + Labels: ars.Labels, + } + + return hash.ComputeTemplateHash(d) +} + func (ars *AutoscalingRunnerSet) ListenerSpecHash() string { arsSpec := ars.Spec.DeepCopy() spec := arsSpec diff --git a/apis/actions.github.com/v1alpha1/ephemeralrunnerset_types.go b/apis/actions.github.com/v1alpha1/ephemeralrunnerset_types.go index 641ec02f..5e8f12d1 100644 --- a/apis/actions.github.com/v1alpha1/ephemeralrunnerset_types.go +++ b/apis/actions.github.com/v1alpha1/ephemeralrunnerset_types.go @@ -23,13 +23,10 @@ import ( // EphemeralRunnerSetSpec defines the desired state of EphemeralRunnerSet type EphemeralRunnerSetSpec struct { // Replicas is the number of desired EphemeralRunner resources in the k8s namespace. - // +optional Replicas int `json:"replicas,omitempty"` // PatchID is the unique identifier for the patch issued by the listener app - // +optional PatchID int `json:"patchID"` // EphemeralRunnerSpec is the spec of the ephemeral runner - // +optional EphemeralRunnerSpec EphemeralRunnerSpec `json:"ephemeralRunnerSpec,omitempty"` // EphemeralRunnerMetadata is the metadata to be applied to all ephemeral runners created by this set. // If the EphemeralRunnerMetadata is updated, the update applies to new ephemeral runners created after the update, @@ -61,13 +58,10 @@ const ( // EphemeralRunnerSet is the Schema for the ephemeralrunnersets API type EphemeralRunnerSet struct { - metav1.TypeMeta `json:",inline"` - // +optional + metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` - // +optional - Spec EphemeralRunnerSetSpec `json:"spec,omitempty"` - // +optional + Spec EphemeralRunnerSetSpec `json:"spec,omitempty"` Status EphemeralRunnerSetStatus `json:"status,omitempty"` } diff --git a/apis/actions.github.com/v1alpha1/proxy_config_test.go b/apis/actions.github.com/v1alpha1/proxy_config_test.go index 0357e5e2..9291cde4 100644 --- a/apis/actions.github.com/v1alpha1/proxy_config_test.go +++ b/apis/actions.github.com/v1alpha1/proxy_config_test.go @@ -14,11 +14,11 @@ import ( func TestProxyConfig_ToSecret(t *testing.T) { config := &v1alpha1.ProxyConfig{ HTTP: &v1alpha1.ProxyServerConfig{ - URL: "http://proxy.example.com:8080", + Url: "http://proxy.example.com:8080", CredentialSecretRef: "my-secret", }, HTTPS: &v1alpha1.ProxyServerConfig{ - URL: "https://proxy.example.com:8080", + Url: "https://proxy.example.com:8080", CredentialSecretRef: "my-secret", }, NoProxy: []string{ @@ -48,11 +48,11 @@ func TestProxyConfig_ToSecret(t *testing.T) { func TestProxyConfig_ProxyFunc(t *testing.T) { config := &v1alpha1.ProxyConfig{ HTTP: &v1alpha1.ProxyServerConfig{ - URL: "http://proxy.example.com:8080", + Url: "http://proxy.example.com:8080", CredentialSecretRef: "my-secret", }, HTTPS: &v1alpha1.ProxyServerConfig{ - URL: "https://proxy.example.com:8080", + Url: "https://proxy.example.com:8080", CredentialSecretRef: "my-secret", }, NoProxy: []string{ diff --git a/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_autoscalinglisteners.yaml b/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_autoscalinglisteners.yaml index 84e243f9..20e57e33 100644 --- a/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_autoscalinglisteners.yaml +++ b/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_autoscalinglisteners.yaml @@ -51,8 +51,10 @@ spec: description: AutoscalingListenerSpec defines the desired state of AutoscalingListener properties: autoscalingRunnerSetName: + description: Required type: string autoscalingRunnerSetNamespace: + description: Required type: string configSecretMetadata: description: ResourceMeta carries metadata common to all internal @@ -68,17 +70,21 @@ spec: type: object type: object ephemeralRunnerSetName: + description: Required type: string githubConfigSecret: + description: Required type: string githubConfigUrl: + description: Required type: string githubServerTLS: properties: certificateFrom: + description: Required properties: configMapKeyRef: - description: Selects a key from a ConfigMap. + description: Required properties: key: description: The key to select. @@ -100,15 +106,13 @@ spec: - key type: object x-kubernetes-map-type: atomic - required: - - configMapKeyRef type: object - required: - - certificateFrom type: object image: + description: Required type: string imagePullSecrets: + description: Required items: description: |- LocalObjectReference contains enough information to let you locate the @@ -127,6 +131,7 @@ spec: x-kubernetes-map-type: atomic type: array maxRunners: + description: Required minimum: 0 type: integer metrics: @@ -178,6 +183,7 @@ spec: type: object type: object minRunners: + description: Required minimum: 0 type: integer proxy: @@ -187,18 +193,16 @@ spec: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object https: properties: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object noProxy: items: @@ -232,7 +236,7 @@ spec: type: object type: object runnerScaleSetId: - minimum: 1 + description: Required type: integer serviceAccountMetadata: description: ResourceMeta carries metadata common to all internal @@ -8778,18 +8782,16 @@ spec: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object https: properties: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object noProxy: items: diff --git a/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_autoscalingrunnersets.yaml b/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_autoscalingrunnersets.yaml index d236b1a9..1f4b63f3 100644 --- a/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_autoscalingrunnersets.yaml +++ b/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_autoscalingrunnersets.yaml @@ -21,21 +21,9 @@ spec: - jsonPath: .spec.maxRunners name: Maximum Runners type: integer - - jsonPath: .status.currentRunners - name: Current Runners - type: integer - jsonPath: .status.phase name: Phase type: string - - jsonPath: .status.pendingEphemeralRunners - name: Pending Runners - type: integer - - jsonPath: .status.runningEphemeralRunners - name: Running Runners - type: integer - - jsonPath: .status.failedEphemeralRunners - name: Failed Runners - type: integer name: v1alpha1 schema: openAPIV3Schema: @@ -110,15 +98,18 @@ spec: type: object type: object githubConfigSecret: + description: Required type: string githubConfigUrl: + description: Required type: string githubServerTLS: properties: certificateFrom: + description: Required properties: configMapKeyRef: - description: Selects a key from a ConfigMap. + description: Required properties: key: description: The key to select. @@ -139,11 +130,7 @@ spec: - key type: object x-kubernetes-map-type: atomic - required: - - configMapKeyRef type: object - required: - - certificateFrom type: object listenerConfigSecretMetadata: description: ResourceMeta carries metadata common to all internal resources @@ -8364,18 +8351,16 @@ spec: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object https: properties: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object noProxy: items: @@ -8391,7 +8376,7 @@ spec: runnerScaleSetName: type: string template: - description: PodTemplateSpec describes the data a pod should have when created from a template + description: Required properties: metadata: description: |- @@ -16517,18 +16502,16 @@ spec: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object https: properties: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object noProxy: items: diff --git a/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_ephemeralrunners.yaml b/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_ephemeralrunners.yaml index 8248263e..3cd90148 100644 --- a/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_ephemeralrunners.yaml +++ b/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_ephemeralrunners.yaml @@ -89,9 +89,10 @@ spec: githubServerTLS: properties: certificateFrom: + description: Required properties: configMapKeyRef: - description: Selects a key from a ConfigMap. + description: Required properties: key: description: The key to select. @@ -112,11 +113,7 @@ spec: - key type: object x-kubernetes-map-type: atomic - required: - - configMapKeyRef type: object - required: - - certificateFrom type: object metadata: description: |- @@ -147,18 +144,16 @@ spec: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object https: properties: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object noProxy: items: @@ -8273,18 +8268,16 @@ spec: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object https: properties: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object noProxy: items: @@ -8297,6 +8290,10 @@ spec: It is used to identify which vault integration should be used to resolve secrets. type: string type: object + required: + - githubConfigSecret + - githubConfigUrl + - runnerScaleSetId type: object status: description: EphemeralRunnerStatus defines the observed state of EphemeralRunner diff --git a/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_ephemeralrunnersets.yaml b/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_ephemeralrunnersets.yaml index 70b21c9a..fa706e3b 100644 --- a/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_ephemeralrunnersets.yaml +++ b/charts/gha-runner-scale-set-controller-experimental/crds/actions.github.com_ephemeralrunnersets.yaml @@ -83,9 +83,10 @@ spec: githubServerTLS: properties: certificateFrom: + description: Required properties: configMapKeyRef: - description: Selects a key from a ConfigMap. + description: Required properties: key: description: The key to select. @@ -106,11 +107,7 @@ spec: - key type: object x-kubernetes-map-type: atomic - required: - - configMapKeyRef type: object - required: - - certificateFrom type: object metadata: description: |- @@ -141,18 +138,16 @@ spec: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object https: properties: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object noProxy: items: @@ -8267,18 +8262,16 @@ spec: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object https: properties: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object noProxy: items: @@ -8291,6 +8284,10 @@ spec: It is used to identify which vault integration should be used to resolve secrets. type: string type: object + required: + - githubConfigSecret + - githubConfigUrl + - runnerScaleSetId type: object patchID: description: PatchID is the unique identifier for the patch issued by the listener app @@ -8298,6 +8295,8 @@ spec: replicas: description: Replicas is the number of desired EphemeralRunner resources in the k8s namespace. type: integer + required: + - patchID type: object status: description: EphemeralRunnerSetStatus defines the observed state of EphemeralRunnerSet diff --git a/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalinglisteners.yaml b/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalinglisteners.yaml index 84e243f9..20e57e33 100644 --- a/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalinglisteners.yaml +++ b/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalinglisteners.yaml @@ -51,8 +51,10 @@ spec: description: AutoscalingListenerSpec defines the desired state of AutoscalingListener properties: autoscalingRunnerSetName: + description: Required type: string autoscalingRunnerSetNamespace: + description: Required type: string configSecretMetadata: description: ResourceMeta carries metadata common to all internal @@ -68,17 +70,21 @@ spec: type: object type: object ephemeralRunnerSetName: + description: Required type: string githubConfigSecret: + description: Required type: string githubConfigUrl: + description: Required type: string githubServerTLS: properties: certificateFrom: + description: Required properties: configMapKeyRef: - description: Selects a key from a ConfigMap. + description: Required properties: key: description: The key to select. @@ -100,15 +106,13 @@ spec: - key type: object x-kubernetes-map-type: atomic - required: - - configMapKeyRef type: object - required: - - certificateFrom type: object image: + description: Required type: string imagePullSecrets: + description: Required items: description: |- LocalObjectReference contains enough information to let you locate the @@ -127,6 +131,7 @@ spec: x-kubernetes-map-type: atomic type: array maxRunners: + description: Required minimum: 0 type: integer metrics: @@ -178,6 +183,7 @@ spec: type: object type: object minRunners: + description: Required minimum: 0 type: integer proxy: @@ -187,18 +193,16 @@ spec: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object https: properties: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object noProxy: items: @@ -232,7 +236,7 @@ spec: type: object type: object runnerScaleSetId: - minimum: 1 + description: Required type: integer serviceAccountMetadata: description: ResourceMeta carries metadata common to all internal @@ -8778,18 +8782,16 @@ spec: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object https: properties: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object noProxy: items: diff --git a/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalingrunnersets.yaml b/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalingrunnersets.yaml index d236b1a9..1f4b63f3 100644 --- a/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalingrunnersets.yaml +++ b/charts/gha-runner-scale-set-controller/crds/actions.github.com_autoscalingrunnersets.yaml @@ -21,21 +21,9 @@ spec: - jsonPath: .spec.maxRunners name: Maximum Runners type: integer - - jsonPath: .status.currentRunners - name: Current Runners - type: integer - jsonPath: .status.phase name: Phase type: string - - jsonPath: .status.pendingEphemeralRunners - name: Pending Runners - type: integer - - jsonPath: .status.runningEphemeralRunners - name: Running Runners - type: integer - - jsonPath: .status.failedEphemeralRunners - name: Failed Runners - type: integer name: v1alpha1 schema: openAPIV3Schema: @@ -110,15 +98,18 @@ spec: type: object type: object githubConfigSecret: + description: Required type: string githubConfigUrl: + description: Required type: string githubServerTLS: properties: certificateFrom: + description: Required properties: configMapKeyRef: - description: Selects a key from a ConfigMap. + description: Required properties: key: description: The key to select. @@ -139,11 +130,7 @@ spec: - key type: object x-kubernetes-map-type: atomic - required: - - configMapKeyRef type: object - required: - - certificateFrom type: object listenerConfigSecretMetadata: description: ResourceMeta carries metadata common to all internal resources @@ -8364,18 +8351,16 @@ spec: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object https: properties: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object noProxy: items: @@ -8391,7 +8376,7 @@ spec: runnerScaleSetName: type: string template: - description: PodTemplateSpec describes the data a pod should have when created from a template + description: Required properties: metadata: description: |- @@ -16517,18 +16502,16 @@ spec: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object https: properties: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object noProxy: items: diff --git a/charts/gha-runner-scale-set-controller/crds/actions.github.com_ephemeralrunners.yaml b/charts/gha-runner-scale-set-controller/crds/actions.github.com_ephemeralrunners.yaml index 8248263e..3cd90148 100644 --- a/charts/gha-runner-scale-set-controller/crds/actions.github.com_ephemeralrunners.yaml +++ b/charts/gha-runner-scale-set-controller/crds/actions.github.com_ephemeralrunners.yaml @@ -89,9 +89,10 @@ spec: githubServerTLS: properties: certificateFrom: + description: Required properties: configMapKeyRef: - description: Selects a key from a ConfigMap. + description: Required properties: key: description: The key to select. @@ -112,11 +113,7 @@ spec: - key type: object x-kubernetes-map-type: atomic - required: - - configMapKeyRef type: object - required: - - certificateFrom type: object metadata: description: |- @@ -147,18 +144,16 @@ spec: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object https: properties: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object noProxy: items: @@ -8273,18 +8268,16 @@ spec: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object https: properties: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object noProxy: items: @@ -8297,6 +8290,10 @@ spec: It is used to identify which vault integration should be used to resolve secrets. type: string type: object + required: + - githubConfigSecret + - githubConfigUrl + - runnerScaleSetId type: object status: description: EphemeralRunnerStatus defines the observed state of EphemeralRunner diff --git a/charts/gha-runner-scale-set-controller/crds/actions.github.com_ephemeralrunnersets.yaml b/charts/gha-runner-scale-set-controller/crds/actions.github.com_ephemeralrunnersets.yaml index 70b21c9a..fa706e3b 100644 --- a/charts/gha-runner-scale-set-controller/crds/actions.github.com_ephemeralrunnersets.yaml +++ b/charts/gha-runner-scale-set-controller/crds/actions.github.com_ephemeralrunnersets.yaml @@ -83,9 +83,10 @@ spec: githubServerTLS: properties: certificateFrom: + description: Required properties: configMapKeyRef: - description: Selects a key from a ConfigMap. + description: Required properties: key: description: The key to select. @@ -106,11 +107,7 @@ spec: - key type: object x-kubernetes-map-type: atomic - required: - - configMapKeyRef type: object - required: - - certificateFrom type: object metadata: description: |- @@ -141,18 +138,16 @@ spec: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object https: properties: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object noProxy: items: @@ -8267,18 +8262,16 @@ spec: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object https: properties: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object noProxy: items: @@ -8291,6 +8284,10 @@ spec: It is used to identify which vault integration should be used to resolve secrets. type: string type: object + required: + - githubConfigSecret + - githubConfigUrl + - runnerScaleSetId type: object patchID: description: PatchID is the unique identifier for the patch issued by the listener app @@ -8298,6 +8295,8 @@ spec: replicas: description: Replicas is the number of desired EphemeralRunner resources in the k8s namespace. type: integer + required: + - patchID type: object status: description: EphemeralRunnerSetStatus defines the observed state of EphemeralRunnerSet diff --git a/charts/gha-runner-scale-set-experimental/templates/_mode_kubernetes.tpl b/charts/gha-runner-scale-set-experimental/templates/_mode_kubernetes.tpl index 672c8495..6589d01d 100644 --- a/charts/gha-runner-scale-set-experimental/templates/_mode_kubernetes.tpl +++ b/charts/gha-runner-scale-set-experimental/templates/_mode_kubernetes.tpl @@ -82,10 +82,7 @@ volumeMounts: subPath: extension readOnly: true {{- end }} - {{- with .Values.runner.container.volumeMounts }} - {{- toYaml . | nindent 2 }} - {{- end }} - {{ include "githubServerTLS.volumeMountItem" (dict "root" $ "existingVolumeMounts" (.Values.runner.container.volumeMounts | default (list))) | nindent 2 }} + {{ include "githubServerTLS.volumeMountItem" (dict "root" $ "existingVolumeMounts" (list)) | nindent 2 }} {{- end }} {{- define "runner-mode-kubernetes.pod-volumes" -}} diff --git a/charts/gha-runner-scale-set-experimental/templates/manager_role.yaml b/charts/gha-runner-scale-set-experimental/templates/manager_role.yaml index a5c9a258..2990ccc4 100644 --- a/charts/gha-runner-scale-set-experimental/templates/manager_role.yaml +++ b/charts/gha-runner-scale-set-experimental/templates/manager_role.yaml @@ -17,8 +17,6 @@ rules: verbs: - create - delete - - update - - patch - get - apiGroups: - "" diff --git a/charts/gha-runner-scale-set/templates/manager_role.yaml b/charts/gha-runner-scale-set/templates/manager_role.yaml index 88ca5a3f..bbf92799 100644 --- a/charts/gha-runner-scale-set/templates/manager_role.yaml +++ b/charts/gha-runner-scale-set/templates/manager_role.yaml @@ -41,8 +41,6 @@ rules: verbs: - create - delete - - update - - patch - get - apiGroups: - "" diff --git a/charts/gha-runner-scale-set/tests/template_test.go b/charts/gha-runner-scale-set/tests/template_test.go index 8c74b956..e9aa4c9b 100644 --- a/charts/gha-runner-scale-set/tests/template_test.go +++ b/charts/gha-runner-scale-set/tests/template_test.go @@ -1365,11 +1365,11 @@ func TestTemplateRenderedWithProxy(t *testing.T) { require.NotNil(t, ars.Spec.Proxy) require.NotNil(t, ars.Spec.Proxy.HTTP) - assert.Equal(t, "http://proxy.example.com", ars.Spec.Proxy.HTTP.URL) + assert.Equal(t, "http://proxy.example.com", ars.Spec.Proxy.HTTP.Url) assert.Equal(t, "http-secret", ars.Spec.Proxy.HTTP.CredentialSecretRef) require.NotNil(t, ars.Spec.Proxy.HTTPS) - assert.Equal(t, "https://proxy.example.com", ars.Spec.Proxy.HTTPS.URL) + assert.Equal(t, "https://proxy.example.com", ars.Spec.Proxy.HTTPS.Url) assert.Equal(t, "https-secret", ars.Spec.Proxy.HTTPS.CredentialSecretRef) require.NotNil(t, ars.Spec.Proxy.NoProxy) diff --git a/config/crd/bases/actions.github.com_autoscalinglisteners.yaml b/config/crd/bases/actions.github.com_autoscalinglisteners.yaml index 84e243f9..20e57e33 100644 --- a/config/crd/bases/actions.github.com_autoscalinglisteners.yaml +++ b/config/crd/bases/actions.github.com_autoscalinglisteners.yaml @@ -51,8 +51,10 @@ spec: description: AutoscalingListenerSpec defines the desired state of AutoscalingListener properties: autoscalingRunnerSetName: + description: Required type: string autoscalingRunnerSetNamespace: + description: Required type: string configSecretMetadata: description: ResourceMeta carries metadata common to all internal @@ -68,17 +70,21 @@ spec: type: object type: object ephemeralRunnerSetName: + description: Required type: string githubConfigSecret: + description: Required type: string githubConfigUrl: + description: Required type: string githubServerTLS: properties: certificateFrom: + description: Required properties: configMapKeyRef: - description: Selects a key from a ConfigMap. + description: Required properties: key: description: The key to select. @@ -100,15 +106,13 @@ spec: - key type: object x-kubernetes-map-type: atomic - required: - - configMapKeyRef type: object - required: - - certificateFrom type: object image: + description: Required type: string imagePullSecrets: + description: Required items: description: |- LocalObjectReference contains enough information to let you locate the @@ -127,6 +131,7 @@ spec: x-kubernetes-map-type: atomic type: array maxRunners: + description: Required minimum: 0 type: integer metrics: @@ -178,6 +183,7 @@ spec: type: object type: object minRunners: + description: Required minimum: 0 type: integer proxy: @@ -187,18 +193,16 @@ spec: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object https: properties: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object noProxy: items: @@ -232,7 +236,7 @@ spec: type: object type: object runnerScaleSetId: - minimum: 1 + description: Required type: integer serviceAccountMetadata: description: ResourceMeta carries metadata common to all internal @@ -8778,18 +8782,16 @@ spec: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object https: properties: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object noProxy: items: diff --git a/config/crd/bases/actions.github.com_autoscalingrunnersets.yaml b/config/crd/bases/actions.github.com_autoscalingrunnersets.yaml index d236b1a9..1f4b63f3 100644 --- a/config/crd/bases/actions.github.com_autoscalingrunnersets.yaml +++ b/config/crd/bases/actions.github.com_autoscalingrunnersets.yaml @@ -21,21 +21,9 @@ spec: - jsonPath: .spec.maxRunners name: Maximum Runners type: integer - - jsonPath: .status.currentRunners - name: Current Runners - type: integer - jsonPath: .status.phase name: Phase type: string - - jsonPath: .status.pendingEphemeralRunners - name: Pending Runners - type: integer - - jsonPath: .status.runningEphemeralRunners - name: Running Runners - type: integer - - jsonPath: .status.failedEphemeralRunners - name: Failed Runners - type: integer name: v1alpha1 schema: openAPIV3Schema: @@ -110,15 +98,18 @@ spec: type: object type: object githubConfigSecret: + description: Required type: string githubConfigUrl: + description: Required type: string githubServerTLS: properties: certificateFrom: + description: Required properties: configMapKeyRef: - description: Selects a key from a ConfigMap. + description: Required properties: key: description: The key to select. @@ -139,11 +130,7 @@ spec: - key type: object x-kubernetes-map-type: atomic - required: - - configMapKeyRef type: object - required: - - certificateFrom type: object listenerConfigSecretMetadata: description: ResourceMeta carries metadata common to all internal resources @@ -8364,18 +8351,16 @@ spec: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object https: properties: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object noProxy: items: @@ -8391,7 +8376,7 @@ spec: runnerScaleSetName: type: string template: - description: PodTemplateSpec describes the data a pod should have when created from a template + description: Required properties: metadata: description: |- @@ -16517,18 +16502,16 @@ spec: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object https: properties: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object noProxy: items: diff --git a/config/crd/bases/actions.github.com_ephemeralrunners.yaml b/config/crd/bases/actions.github.com_ephemeralrunners.yaml index 8248263e..3cd90148 100644 --- a/config/crd/bases/actions.github.com_ephemeralrunners.yaml +++ b/config/crd/bases/actions.github.com_ephemeralrunners.yaml @@ -89,9 +89,10 @@ spec: githubServerTLS: properties: certificateFrom: + description: Required properties: configMapKeyRef: - description: Selects a key from a ConfigMap. + description: Required properties: key: description: The key to select. @@ -112,11 +113,7 @@ spec: - key type: object x-kubernetes-map-type: atomic - required: - - configMapKeyRef type: object - required: - - certificateFrom type: object metadata: description: |- @@ -147,18 +144,16 @@ spec: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object https: properties: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object noProxy: items: @@ -8273,18 +8268,16 @@ spec: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object https: properties: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object noProxy: items: @@ -8297,6 +8290,10 @@ spec: It is used to identify which vault integration should be used to resolve secrets. type: string type: object + required: + - githubConfigSecret + - githubConfigUrl + - runnerScaleSetId type: object status: description: EphemeralRunnerStatus defines the observed state of EphemeralRunner diff --git a/config/crd/bases/actions.github.com_ephemeralrunnersets.yaml b/config/crd/bases/actions.github.com_ephemeralrunnersets.yaml index 70b21c9a..fa706e3b 100644 --- a/config/crd/bases/actions.github.com_ephemeralrunnersets.yaml +++ b/config/crd/bases/actions.github.com_ephemeralrunnersets.yaml @@ -83,9 +83,10 @@ spec: githubServerTLS: properties: certificateFrom: + description: Required properties: configMapKeyRef: - description: Selects a key from a ConfigMap. + description: Required properties: key: description: The key to select. @@ -106,11 +107,7 @@ spec: - key type: object x-kubernetes-map-type: atomic - required: - - configMapKeyRef type: object - required: - - certificateFrom type: object metadata: description: |- @@ -141,18 +138,16 @@ spec: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object https: properties: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object noProxy: items: @@ -8267,18 +8262,16 @@ spec: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object https: properties: credentialSecretRef: type: string url: + description: Required type: string - required: - - url type: object noProxy: items: @@ -8291,6 +8284,10 @@ spec: It is used to identify which vault integration should be used to resolve secrets. type: string type: object + required: + - githubConfigSecret + - githubConfigUrl + - runnerScaleSetId type: object patchID: description: PatchID is the unique identifier for the patch issued by the listener app @@ -8298,6 +8295,8 @@ spec: replicas: description: Replicas is the number of desired EphemeralRunner resources in the k8s namespace. type: integer + required: + - patchID type: object status: description: EphemeralRunnerSetStatus defines the observed state of EphemeralRunnerSet diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 007b8516..dc0becfd 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -52,7 +52,7 @@ rules: - delete - get - list - - patch + - update - watch - apiGroups: - actions.github.com @@ -167,5 +167,5 @@ rules: - delete - get - list - - patch + - update - watch diff --git a/controllers/actions.github.com/autoscalinglistener_controller.go b/controllers/actions.github.com/autoscalinglistener_controller.go index a62462e9..c0aa8111 100644 --- a/controllers/actions.github.com/autoscalinglistener_controller.go +++ b/controllers/actions.github.com/autoscalinglistener_controller.go @@ -62,10 +62,10 @@ type AutoscalingListenerReconciler struct { // +kubebuilder:rbac:groups=core,resources=pods,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=core,resources=pods/status,verbs=get -// +kubebuilder:rbac:groups=core,resources=secrets,verbs=get;list;watch;create;patch -// +kubebuilder:rbac:groups=core,resources=serviceaccounts,verbs=get;list;watch;create;patch -// +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=roles,verbs=create;delete;get;list;watch;patch -// +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=rolebindings,verbs=create;delete;get;list;watch;patch +// +kubebuilder:rbac:groups=core,resources=secrets,verbs=get;list;watch;create;update +// +kubebuilder:rbac:groups=core,resources=serviceaccounts,verbs=get;list;watch;create;update +// +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=roles,verbs=create;delete;get;list;watch;update +// +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=rolebindings,verbs=create;delete;get;list;watch;update // +kubebuilder:rbac:groups=actions.github.com,resources=autoscalinglisteners,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=actions.github.com,resources=autoscalinglisteners/status,verbs=get;update;patch // +kubebuilder:rbac:groups=actions.github.com,resources=autoscalinglisteners/finalizers,verbs=update @@ -163,19 +163,18 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. return ctrl.Result{}, err } - updatedServiceAccount := serviceAccount.DeepCopy() - var shouldUpdate bool desiredLabels := r.filterAndMergeLabels(serviceAccount.Labels, desiredServiceAccount.Labels) - if !maps.Equal(serviceAccount.Labels, desiredLabels) { - updatedServiceAccount.Labels = desiredLabels - shouldUpdate = true - } - desiredAnnotations := r.filterAndMergeAnnotations(serviceAccount.Annotations, desiredServiceAccount.Annotations) - if !r.annotationsEqual(serviceAccount.Annotations, desiredAnnotations) { - updatedServiceAccount.Annotations = desiredAnnotations - shouldUpdate = true - } - if shouldUpdate { + labelsModified := !maps.Equal(serviceAccount.Labels, desiredLabels) + desiredAnnotations := r.mergeAnnotations(serviceAccount.Annotations, desiredServiceAccount.Annotations) + annotationsModified := !maps.Equal(serviceAccount.Annotations, desiredAnnotations) + if labelsModified || annotationsModified { + updatedServiceAccount := serviceAccount.DeepCopy() + if labelsModified { + updatedServiceAccount.Labels = desiredLabels + } + if annotationsModified { + updatedServiceAccount.Annotations = desiredAnnotations + } log.Info("Updating listener service account") if err := r.Patch(ctx, updatedServiceAccount, client.MergeFrom(&serviceAccount)); err != nil { @@ -207,23 +206,22 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. switch { case err == nil: desiredRole := r.newScaleSetListenerRole(&autoscalingListener) - updatedRole := listenerRole.DeepCopy() - var shouldUpdate bool desiredLabels := r.filterAndMergeLabels(listenerRole.Labels, desiredRole.Labels) - if !maps.Equal(listenerRole.Labels, desiredLabels) { - updatedRole.Labels = desiredLabels - shouldUpdate = true - } - desiredAnnotations := r.filterAndMergeAnnotations(listenerRole.Annotations, desiredRole.Annotations) - if !r.annotationsEqual(listenerRole.Annotations, desiredAnnotations) { - updatedRole.Annotations = desiredAnnotations - shouldUpdate = true - } - if !reflect.DeepEqual(listenerRole.Rules, desiredRole.Rules) { - updatedRole.Rules = desiredRole.Rules - shouldUpdate = true - } - if shouldUpdate { + labelsModified := !maps.Equal(listenerRole.Labels, desiredLabels) + desiredAnnotations := r.mergeAnnotations(listenerRole.Annotations, desiredRole.Annotations) + annotationsModified := !maps.Equal(listenerRole.Annotations, desiredAnnotations) + rulesModified := !reflect.DeepEqual(listenerRole.Rules, desiredRole.Rules) + if labelsModified || annotationsModified || rulesModified { + updatedRole := listenerRole.DeepCopy() + if labelsModified { + updatedRole.Labels = desiredLabels + } + if annotationsModified { + updatedRole.Annotations = desiredAnnotations + } + if rulesModified { + updatedRole.Rules = desiredRole.Rules + } log.Info("Updating listener role") if err := r.Patch(ctx, updatedRole, client.MergeFrom(&listenerRole)); err != nil { log.Error(err, "Failed to update listener role") @@ -250,19 +248,18 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. &listenerRole, &serviceAccount, ) - updatedRoleBinding := listenerRoleBinding.DeepCopy() - var shouldUpdate bool desiredLabels := r.filterAndMergeLabels(listenerRoleBinding.Labels, desiredRoleBinding.Labels) - if !maps.Equal(listenerRoleBinding.Labels, desiredLabels) { - updatedRoleBinding.Labels = desiredLabels - shouldUpdate = true - } - desiredAnnotations := r.filterAndMergeAnnotations(listenerRoleBinding.Annotations, desiredRoleBinding.Annotations) - if !r.annotationsEqual(listenerRoleBinding.Annotations, desiredAnnotations) { - updatedRoleBinding.Annotations = desiredAnnotations - shouldUpdate = true - } - if shouldUpdate { + labelsModified := !maps.Equal(listenerRoleBinding.Labels, desiredLabels) + desiredAnnotations := r.mergeAnnotations(listenerRoleBinding.Annotations, desiredRoleBinding.Annotations) + annotationsModified := !maps.Equal(listenerRoleBinding.Annotations, desiredAnnotations) + if labelsModified || annotationsModified { + updatedRoleBinding := listenerRoleBinding.DeepCopy() + if labelsModified { + updatedRoleBinding.Labels = desiredLabels + } + if annotationsModified { + updatedRoleBinding.Annotations = desiredAnnotations + } log.Info("Updating listener role binding") if err := r.Patch(ctx, updatedRoleBinding, client.MergeFrom(&listenerRoleBinding)); err != nil { log.Error(err, "Failed to update listener role binding") @@ -306,19 +303,18 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. log.Error(err, "Failed to build desired listener proxy secret") return ctrl.Result{}, err } - updatedProxySecret := proxySecret.DeepCopy() - var shouldUpdate bool desiredLabels := r.filterAndMergeLabels(proxySecret.Labels, desiredListenerProxy.Labels) - if !maps.Equal(proxySecret.Labels, desiredLabels) { - updatedProxySecret.Labels = desiredLabels - shouldUpdate = true - } - desiredAnnotations := r.filterAndMergeAnnotations(proxySecret.Annotations, desiredListenerProxy.Annotations) - if !r.annotationsEqual(proxySecret.Annotations, desiredAnnotations) { - updatedProxySecret.Annotations = desiredAnnotations - shouldUpdate = true - } - if shouldUpdate { + labelsModified := !maps.Equal(proxySecret.Labels, desiredLabels) + desiredAnnotations := r.mergeAnnotations(proxySecret.Annotations, desiredListenerProxy.Annotations) + annotationsModified := !maps.Equal(proxySecret.Annotations, desiredAnnotations) + if labelsModified || annotationsModified { + updatedProxySecret := proxySecret.DeepCopy() + if labelsModified { + updatedProxySecret.Labels = desiredLabels + } + if annotationsModified { + updatedProxySecret.Annotations = desiredAnnotations + } log.Info("Updating listener proxy secret") if err := r.Patch(ctx, updatedProxySecret, client.MergeFrom(&proxySecret)); err != nil { log.Error(err, "Failed to update listener proxy secret") @@ -393,20 +389,19 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. if err != nil { return ctrl.Result{}, fmt.Errorf("failed to build listener config secret: %w", err) } - updatedSecret := listenerConfigSecret.DeepCopy() - var shouldUpdate bool desiredLabels := r.filterAndMergeLabels(listenerConfigSecret.Labels, desiredSecret.Labels) - if !maps.Equal(listenerConfigSecret.Labels, desiredLabels) { - updatedSecret.Labels = desiredLabels - shouldUpdate = true - } - desiredAnnotations := r.filterAndMergeAnnotations(listenerConfigSecret.Annotations, desiredSecret.Annotations) - if !r.annotationsEqual(listenerConfigSecret.Annotations, desiredAnnotations) { - updatedSecret.Annotations = desiredAnnotations - shouldUpdate = true - } + labelsModified := !maps.Equal(listenerConfigSecret.Labels, desiredLabels) + desiredAnnotations := r.mergeAnnotations(listenerConfigSecret.Annotations, desiredSecret.Annotations) + annotationsModified := !maps.Equal(listenerConfigSecret.Annotations, desiredAnnotations) - if shouldUpdate { + if labelsModified || annotationsModified { + updatedSecret := listenerConfigSecret.DeepCopy() + if labelsModified { + updatedSecret.Labels = desiredLabels + } + if annotationsModified { + updatedSecret.Annotations = desiredAnnotations + } log.Info("Updating listener config secret", "namespace", updatedSecret.Namespace, "name", updatedSecret.Name) if err := r.Patch(ctx, updatedSecret, client.MergeFrom(&listenerConfigSecret)); err != nil { return ctrl.Result{}, fmt.Errorf("failed to update listener config secret: %w", err) @@ -467,17 +462,9 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. return ctrl.Result{}, err } - if desiredPod.Annotations[AnnotationKeyIntegrityHash] != listenerPod.Annotations[AnnotationKeyIntegrityHash] { - // Since the pod is controlled by a pod controller, we tag the pod with integrity hash. - // If the integrity hash is changed, that means the new spec is different. Keep in mind, the tagged hash - // is created by hashing only the fields this controller sets. - log.Info( - "Listener pod dependency changed, recreating listener pod", - "desiredSpec", - mustJSON(desiredPod.Spec), - "currentSpec", - mustJSON(listenerPod.Spec), - ) + shouldReCreate := desiredPod.Annotations[annotationKeyIntegrityHash] != listenerPod.Annotations[annotationKeyIntegrityHash] + if shouldReCreate { + log.Info("Listener pod dependency changed, recreating listener pod") if err := r.deleteListenerPod(ctx, &autoscalingListener, &listenerPod, log); err != nil { return ctrl.Result{}, err } @@ -486,20 +473,19 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. return ctrl.Result{}, nil } - updatedPod := listenerPod.DeepCopy() - var shouldUpdate bool desiredLabels := r.filterAndMergeLabels(listenerPod.Labels, desiredPod.Labels) - if !maps.Equal(listenerPod.Labels, desiredLabels) { - updatedPod.Labels = desiredLabels - shouldUpdate = true - } - desiredAnnotations := r.filterAndMergeAnnotations(listenerPod.Annotations, desiredPod.Annotations) - if !r.annotationsEqual(listenerPod.Annotations, desiredAnnotations) { - updatedPod.Annotations = desiredAnnotations - shouldUpdate = true - } + labelsModified := !maps.Equal(listenerPod.Labels, desiredLabels) + desiredAnnotations := r.mergeAnnotations(listenerPod.Annotations, desiredPod.Annotations) + annotationsModified := !maps.Equal(listenerPod.Annotations, desiredAnnotations) - if shouldUpdate { + if labelsModified || annotationsModified { + updatedPod := listenerPod.DeepCopy() + if labelsModified { + updatedPod.Labels = desiredLabels + } + if annotationsModified { + updatedPod.Annotations = desiredAnnotations + } log.Info("Updating listener pod", "namespace", updatedPod.Namespace, "name", updatedPod.Name) if err := r.Patch(ctx, updatedPod, client.MergeFrom(&listenerPod)); err != nil { log.Error(err, "Unable to update listener pod", "namespace", updatedPod.Namespace, "name", updatedPod.Name) @@ -527,11 +513,7 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. return ctrl.Result{}, err } - log.Info( - "Creating listener pod", - "namespace", desiredPod.Namespace, - "name", desiredPod.Name, - ) + log.Info("Creating listener pod", "namespace", desiredPod.Namespace, "name", desiredPod.Name) if err := r.Create(ctx, desiredPod); err != nil { log.Error(err, "Unable to create listener pod", "namespace", desiredPod.Namespace, "name", desiredPod.Name) return ctrl.Result{}, err diff --git a/controllers/actions.github.com/autoscalinglistener_controller_test.go b/controllers/actions.github.com/autoscalinglistener_controller_test.go index 1d895806..d48d613e 100644 --- a/controllers/actions.github.com/autoscalinglistener_controller_test.go +++ b/controllers/actions.github.com/autoscalinglistener_controller_test.go @@ -516,7 +516,7 @@ var _ = Describe("Test AutoScalingListener controller", func() { }, }, } - err := k8sClient.Status().Patch(ctx, updated, client.MergeFrom(pod)) + err := k8sClient.Status().Update(ctx, updated) Expect(err).NotTo(HaveOccurred(), "failed to update test pod") // Waiting for the new pod is created @@ -785,7 +785,7 @@ var _ = Describe("Test AutoScalingListener customization", func() { }, }, } - err := k8sClient.Status().Patch(ctx, updated, client.MergeFrom(pod)) + err := k8sClient.Status().Update(ctx, updated) Expect(err).NotTo(HaveOccurred(), "failed to update pod status") pod = new(corev1.Pod) @@ -831,7 +831,7 @@ var _ = Describe("Test AutoScalingListener customization", func() { updated := pod.DeepCopy() oldPodUID := string(pod.UID) updated.Status.Reason = "Evicted" - err := k8sClient.Status().Patch(ctx, updated, client.MergeFrom(pod)) + err := k8sClient.Status().Update(ctx, updated) Expect(err).NotTo(HaveOccurred(), "failed to update pod status") pod = new(corev1.Pod) @@ -954,11 +954,11 @@ var _ = Describe("Test AutoScalingListener controller with proxy", func() { proxy := &v1alpha1.ProxyConfig{ HTTP: &v1alpha1.ProxyServerConfig{ - URL: "http://localhost:8080", + Url: "http://localhost:8080", CredentialSecretRef: "proxy-credentials", }, HTTPS: &v1alpha1.ProxyServerConfig{ - URL: "https://localhost:8443", + Url: "https://localhost:8443", CredentialSecretRef: "proxy-credentials", }, NoProxy: []string{ diff --git a/controllers/actions.github.com/autoscalingrunnerset_controller.go b/controllers/actions.github.com/autoscalingrunnerset_controller.go index 14df0d61..d1913f27 100644 --- a/controllers/actions.github.com/autoscalingrunnerset_controller.go +++ b/controllers/actions.github.com/autoscalingrunnerset_controller.go @@ -142,12 +142,13 @@ func (r *AutoscalingRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl } // Something has changed, we need to re-apply the pending phase and change hash annotation to trigger the update of runner scale set and listener. - if targetHash := autoscalingRunnerSetIntegrityHash(&autoscalingRunnerSet); autoscalingRunnerSet.Annotations[AnnotationKeyIntegrityHash] != targetHash { + if targetHash := autoscalingRunnerSet.Hash(); autoscalingRunnerSet.Annotations[annotationKeyIntegrityHash] != targetHash { + // TODO: apply the version label original := autoscalingRunnerSet.DeepCopy() if autoscalingRunnerSet.Annotations == nil { autoscalingRunnerSet.Annotations = map[string]string{} } - autoscalingRunnerSet.Annotations[AnnotationKeyIntegrityHash] = targetHash + autoscalingRunnerSet.Annotations[annotationKeyIntegrityHash] = targetHash if err := r.Patch(ctx, &autoscalingRunnerSet, client.MergeFrom(original)); err != nil { log.Error(err, "Failed to update autoscaling runner set with new change hash and pending phase") return ctrl.Result{}, err @@ -290,13 +291,34 @@ func (r *AutoscalingRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl return ctrl.Result{}, nil } - integrityDiff := desired.Annotations[AnnotationKeyIntegrityHash] != ephemeralRunnerSetIntegrityHash(&ephemeralRunnerSet) - if integrityDiff { + if ephemeralRunnerSet.Annotations[annotationKeyIntegrityHash] != desired.Annotations[annotationKeyIntegrityHash] { + // When runners are actively processing jobs, defer the spec update: + // delete the listener to stop accepting new jobs, but leave the ERS + // (and its running pods) untouched until all jobs have drained. + var ephemeralRunnerList v1alpha1.EphemeralRunnerList + if err := r.List(ctx, &ephemeralRunnerList, + client.InNamespace(ephemeralRunnerSet.Namespace), + client.MatchingFields{resourceOwnerKey: ephemeralRunnerSet.Name}, + ); err != nil { + log.Error(err, "Failed to list ephemeral runners") + return ctrl.Result{}, err + } + + ephemeralRunnersByState := newEphemeralRunnersByStates(&ephemeralRunnerList) + if len(ephemeralRunnersByState.running)+len(ephemeralRunnersByState.pending) > 0 { + log.Info("Ephemeral runner set spec changed but runners are still active; deleting listener to stop new jobs") + if _, err := r.cleanupListener(ctx, &autoscalingRunnerSet, log); err != nil { + log.Error(err, "Failed to clean up listener while waiting for runners to drain") + return ctrl.Result{}, err + } + return ctrl.Result{RequeueAfter: 1 * time.Second}, nil + } + original := ephemeralRunnerSet.DeepCopy() ephemeralRunnerSet.Spec.EphemeralRunnerMetadata = desired.Spec.EphemeralRunnerMetadata ephemeralRunnerSet.Spec.EphemeralRunnerSpec = desired.Spec.EphemeralRunnerSpec ephemeralRunnerSet.Labels = r.filterAndMergeLabels(ephemeralRunnerSet.Labels, desired.Labels) - ephemeralRunnerSet.Annotations = r.filterAndMergeAnnotations(ephemeralRunnerSet.Annotations, desired.Annotations) + ephemeralRunnerSet.Annotations = r.mergeAnnotations(ephemeralRunnerSet.Annotations, desired.Annotations) log.Info("Updating ephemeral runner set spec to match the desired spec") if err := r.Patch(ctx, &ephemeralRunnerSet, client.MergeFrom(original)); err != nil { @@ -310,12 +332,12 @@ func (r *AutoscalingRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl ephemeralRunnerMetadataModified := !cmp.Equal(ephemeralRunnerSet.Spec.EphemeralRunnerMetadata, desired.Spec.EphemeralRunnerMetadata) ephemeralRunnerLabelsModified := !maps.Equal(ephemeralRunnerSet.Labels, desired.Labels) - ephemeralRunnerAnnotationsModified := !r.annotationsEqual(ephemeralRunnerSet.Annotations, desired.Annotations) + ephemeralRunnerAnnotationsModified := !maps.Equal(ephemeralRunnerSet.Annotations, desired.Annotations) if ephemeralRunnerLabelsModified || ephemeralRunnerAnnotationsModified || ephemeralRunnerMetadataModified { original := ephemeralRunnerSet.DeepCopy() ephemeralRunnerSet.Labels = r.filterAndMergeLabels(ephemeralRunnerSet.Labels, desired.Labels) - ephemeralRunnerSet.Annotations = r.filterAndMergeAnnotations(ephemeralRunnerSet.Annotations, desired.Annotations) + ephemeralRunnerSet.Annotations = r.mergeAnnotations(ephemeralRunnerSet.Annotations, desired.Annotations) ephemeralRunnerSet.Spec.EphemeralRunnerMetadata = desired.Spec.EphemeralRunnerMetadata log.Info("Updating ephemeral runner set metadata to match desired labels and annotations") if err := r.Patch(ctx, &ephemeralRunnerSet, client.MergeFrom(original)); err != nil { @@ -358,18 +380,14 @@ func (r *AutoscalingRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl } if !cmp.Equal(listener.Spec, desired.Spec) || - !maps.Equal(listener.Labels, desired.Labels) || - !r.annotationsEqual(listener.Annotations, desired.Annotations) { - log.Info("Updating listener") - original := listener.DeepCopy() - listener.Spec = desired.Spec - listener.Annotations = r.filterAndMergeAnnotations(listener.Annotations, desired.Annotations) - listener.Labels = r.filterAndMergeLabels(listener.Labels, desired.Labels) - if err := r.Patch(ctx, &listener, client.MergeFrom(original)); err != nil { - log.Error(err, "Failed to update AutoscalingListener with new spec") + !cmp.Equal(listener.Labels, desired.Labels) || + !cmp.Equal(listener.Annotations, desired.Annotations) { + log.Info("Deleting AutoscalingListener to re-create with updated spec") + if err := r.Delete(ctx, &listener); err != nil { + log.Error(err, "Failed to delete AutoscalingListener for re-creation") return ctrl.Result{}, err } - log.Info("Successfully updated AutoscalingListener with new spec") + log.Info("Deleted AutoscalingListener, will re-create on next reconcile") return ctrl.Result{}, nil } } diff --git a/controllers/actions.github.com/autoscalingrunnerset_controller_test.go b/controllers/actions.github.com/autoscalingrunnerset_controller_test.go index 400ebcdc..e7bb6f08 100644 --- a/controllers/actions.github.com/autoscalingrunnerset_controller_test.go +++ b/controllers/actions.github.com/autoscalingrunnerset_controller_test.go @@ -461,14 +461,7 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { listener := new(v1alpha1.AutoscalingListener) Eventually( func() error { - return k8sClient.Get( - ctx, - client.ObjectKey{ - Name: scaleSetListenerName(autoscalingRunnerSet), - Namespace: autoscalingRunnerSet.Namespace, - }, - listener, - ) + return k8sClient.Get(ctx, client.ObjectKey{Name: scaleSetListenerName(autoscalingRunnerSet), Namespace: autoscalingRunnerSet.Namespace}, listener) }, autoscalingRunnerSetTestTimeout, autoscalingRunnerSetTestInterval, @@ -479,21 +472,13 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { runnerSet := new(v1alpha1.EphemeralRunnerSet) Eventually( func() error { - return k8sClient.Get( - ctx, - client.ObjectKey{ - Name: autoscalingRunnerSet.Name, - Namespace: autoscalingRunnerSet.Namespace, - }, - runnerSet, - ) + return k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingRunnerSet.Name, Namespace: autoscalingRunnerSet.Namespace}, runnerSet) }, autoscalingRunnerSetTestTimeout, autoscalingRunnerSetTestInterval, ).Should(Succeed(), "EphemeralRunnerSet should be created") originalRunnerSetUID := runnerSet.UID - originalRunnerSetHash := runnerSet.Annotations[AnnotationKeyIntegrityHash] - originalResourceVersion := runnerSet.ResourceVersion + originalRunnerSetHash := runnerSet.Annotations[annotationKeyIntegrityHash] patched := autoscalingRunnerSet.DeepCopy() patched.Spec.Template.Spec.Containers[0].Image = "ghcr.io/actions/runner:updated" @@ -507,8 +492,7 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { g.Expect(err).NotTo(HaveOccurred(), "failed to get EphemeralRunnerSet") g.Expect(current.UID).To(Equal(originalRunnerSetUID), "EphemeralRunnerSet should be updated in place") g.Expect(current.Spec.EphemeralRunnerSpec.PodTemplateSpec.Spec.Containers[0].Image).To(Equal("ghcr.io/actions/runner:updated")) - g.Expect(current.Annotations[AnnotationKeyIntegrityHash]).To(Equal(originalRunnerSetHash), "EphemeralRunnerSet hash integrity key should not be modified") - g.Expect(current.ResourceVersion).NotTo(Equal(originalResourceVersion), "EphemeralRunnerSet ResourceVersion should change after update") + g.Expect(current.Annotations[annotationKeyIntegrityHash]).NotTo(Equal(originalRunnerSetHash), "EphemeralRunnerSet spec hash should change") }, autoscalingRunnerSetTestTimeout, autoscalingRunnerSetTestInterval, @@ -520,50 +504,34 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { err := k8sClient.Get(ctx, client.ObjectKey{Name: scaleSetListenerName(autoscalingRunnerSet), Namespace: autoscalingRunnerSet.Namespace}, current) g.Expect(err).NotTo(HaveOccurred(), "failed to get Listener") g.Expect(current.UID).To(Equal(originalListenerUID), "Listener should not be recreated") - g.Expect(current.ResourceVersion).To(Equal(originalListenerResourceVersion), "Listener ResourceVersion should not change after update") + g.Expect(current.ResourceVersion).To(Equal(originalListenerResourceVersion), "Listener should not be updated") }, - autoscalingRunnerSetTestTimeout, + time.Second*5, autoscalingRunnerSetTestInterval, ).Should(Succeed()) }) - It("Updates only the Listener when max runners changes", func() { + It("recreates only the Listener when max runners changes", func() { listener := new(v1alpha1.AutoscalingListener) Eventually( func() error { - return k8sClient.Get( - ctx, - client.ObjectKey{ - Name: scaleSetListenerName(autoscalingRunnerSet), - Namespace: autoscalingRunnerSet.Namespace, - }, - listener, - ) + return k8sClient.Get(ctx, client.ObjectKey{Name: scaleSetListenerName(autoscalingRunnerSet), Namespace: autoscalingRunnerSet.Namespace}, listener) }, autoscalingRunnerSetTestTimeout, autoscalingRunnerSetTestInterval, ).Should(Succeed(), "Listener should be created") originalListenerUID := listener.UID - originalListenerResourceVersion := listener.ResourceVersion - originalListenerIntegrityHash := listener.Annotations[AnnotationKeyIntegrityHash] runnerSet := new(v1alpha1.EphemeralRunnerSet) Eventually( func() error { - return k8sClient.Get( - ctx, - client.ObjectKey{ - Name: autoscalingRunnerSet.Name, - Namespace: autoscalingRunnerSet.Namespace, - }, - runnerSet, - ) + return k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingRunnerSet.Name, Namespace: autoscalingRunnerSet.Namespace}, runnerSet) }, autoscalingRunnerSetTestTimeout, autoscalingRunnerSetTestInterval, ).Should(Succeed(), "EphemeralRunnerSet should be created") - originalERSRunnerSetUID := runnerSet.UID - originalERSResourceVersion := runnerSet.ResourceVersion + originalRunnerSetUID := runnerSet.UID + originalRunnerSetHash := runnerSet.Annotations[annotationKeyIntegrityHash] patched := autoscalingRunnerSet.DeepCopy() max := 20 @@ -576,10 +544,8 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { current := new(v1alpha1.AutoscalingListener) err := k8sClient.Get(ctx, client.ObjectKey{Name: scaleSetListenerName(autoscalingRunnerSet), Namespace: autoscalingRunnerSet.Namespace}, current) g.Expect(err).NotTo(HaveOccurred(), "failed to get Listener") - g.Expect(current.UID).To(Equal(originalListenerUID), "Listener should be updated") - g.Expect(current.Annotations[AnnotationKeyIntegrityHash]).To(Equal(originalListenerIntegrityHash), "Listener hash integrity key should not be modified") + g.Expect(current.UID).NotTo(Equal(originalListenerUID), "Listener should be recreated") g.Expect(current.Spec.MaxRunners).To(Equal(max)) - g.Expect(current.ResourceVersion).NotTo(Equal(originalListenerResourceVersion), "Listener ResourceVersion should change after update") }, autoscalingRunnerSetTestTimeout, autoscalingRunnerSetTestInterval, @@ -590,8 +556,8 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { current := new(v1alpha1.EphemeralRunnerSet) err := k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingRunnerSet.Name, Namespace: autoscalingRunnerSet.Namespace}, current) g.Expect(err).NotTo(HaveOccurred(), "failed to get EphemeralRunnerSet") - g.Expect(current.UID).To(Equal(originalERSRunnerSetUID), "EphemeralRunnerSet should not be recreated") - g.Expect(current.ResourceVersion).To(Equal(originalERSResourceVersion), "EphemeralRunnerSet spec should not change") + g.Expect(current.UID).To(Equal(originalRunnerSetUID), "EphemeralRunnerSet should not be recreated") + g.Expect(current.Annotations[annotationKeyIntegrityHash]).To(Equal(originalRunnerSetHash), "EphemeralRunnerSet spec should not change") }, time.Second*5, autoscalingRunnerSetTestInterval, @@ -602,14 +568,7 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { runnerSet := new(v1alpha1.EphemeralRunnerSet) Eventually( func() (string, error) { - err := k8sClient.Get( - ctx, - client.ObjectKey{ - Name: autoscalingRunnerSet.Name, - Namespace: autoscalingRunnerSet.Namespace, - }, - runnerSet, - ) + err := k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingRunnerSet.Name, Namespace: autoscalingRunnerSet.Namespace}, runnerSet) if err != nil { return "", err } @@ -627,14 +586,7 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { Eventually( func() (string, error) { current := new(v1alpha1.EphemeralRunnerSet) - err := k8sClient.Get( - ctx, - client.ObjectKey{ - Name: autoscalingRunnerSet.Name, - Namespace: autoscalingRunnerSet.Namespace, - }, - current, - ) + err := k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingRunnerSet.Name, Namespace: autoscalingRunnerSet.Namespace}, current) if err != nil { return "", err } @@ -649,14 +601,7 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { runnerSet := new(v1alpha1.EphemeralRunnerSet) Eventually( func() (string, error) { - err := k8sClient.Get( - ctx, - client.ObjectKey{ - Name: autoscalingRunnerSet.Name, - Namespace: autoscalingRunnerSet.Namespace, - }, - runnerSet, - ) + err := k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingRunnerSet.Name, Namespace: autoscalingRunnerSet.Namespace}, runnerSet) if err != nil { return "", err } @@ -669,8 +614,6 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { patched := autoscalingRunnerSet.DeepCopy() patched.Spec.EphemeralRunnerSetMetadata.Annotations["arc.test/metadata-annotation"] = "updated" patched.Spec.EphemeralRunnerSetMetadata.Annotations["arc.test/new-metadata-annotation"] = "added" - originalERSIntegrityHash := runnerSet.Annotations[AnnotationKeyIntegrityHash] - patched.Spec.EphemeralRunnerSetMetadata.Annotations[AnnotationKeyIntegrityHash] = "must-not-be-modified" err := k8sClient.Patch(ctx, patched, client.MergeFrom(autoscalingRunnerSet)) Expect(err).NotTo(HaveOccurred(), "failed to patch AutoScalingRunnerSet EphemeralRunnerSet metadata") @@ -681,7 +624,6 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { g.Expect(err).NotTo(HaveOccurred(), "failed to get EphemeralRunnerSet") g.Expect(current.Annotations["arc.test/metadata-annotation"]).To(Equal("updated")) g.Expect(current.Annotations["arc.test/new-metadata-annotation"]).To(Equal("added")) - g.Expect(current.Annotations[AnnotationKeyIntegrityHash]).To(Equal(originalERSIntegrityHash), "EphemeralRunnerSet hash integrity key should not be modified") }, autoscalingRunnerSetTestTimeout, autoscalingRunnerSetTestInterval, @@ -692,14 +634,7 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { runnerSet := new(v1alpha1.EphemeralRunnerSet) Eventually( func(g Gomega) { - err := k8sClient.Get( - ctx, - client.ObjectKey{ - Name: autoscalingRunnerSet.Name, - Namespace: autoscalingRunnerSet.Namespace, - }, - runnerSet, - ) + err := k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingRunnerSet.Name, Namespace: autoscalingRunnerSet.Namespace}, runnerSet) g.Expect(err).NotTo(HaveOccurred(), "failed to get EphemeralRunnerSet") g.Expect(runnerSet.Spec.EphemeralRunnerMetadata).NotTo(BeNil()) g.Expect(runnerSet.Spec.EphemeralRunnerMetadata.Labels["arc.test/runner-metadata-label"]).To(Equal("initial")) @@ -784,38 +719,22 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { listener := new(v1alpha1.AutoscalingListener) Eventually( func() error { - return k8sClient.Get( - ctx, - client.ObjectKey{ - Name: scaleSetListenerName(autoscalingRunnerSet), - Namespace: autoscalingRunnerSet.Namespace, - }, - listener, - ) + return k8sClient.Get(ctx, client.ObjectKey{Name: scaleSetListenerName(autoscalingRunnerSet), Namespace: autoscalingRunnerSet.Namespace}, listener) }, autoscalingRunnerSetTestTimeout, autoscalingRunnerSetTestInterval, ).Should(Succeed(), "Listener should be created") originalListenerUID := listener.UID - originalListenerIntegrityHash := listener.Annotations[AnnotationKeyIntegrityHash] runnerSet := new(v1alpha1.EphemeralRunnerSet) Eventually( func() error { - return k8sClient.Get( - ctx, - client.ObjectKey{ - Name: autoscalingRunnerSet.Name, - Namespace: autoscalingRunnerSet.Namespace, - }, - runnerSet, - ) + return k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingRunnerSet.Name, Namespace: autoscalingRunnerSet.Namespace}, runnerSet) }, autoscalingRunnerSetTestTimeout, autoscalingRunnerSetTestInterval, ).Should(Succeed(), "EphemeralRunnerSet should be created") - originalEphemeralRunnerSetUID := runnerSet.UID - originalEphemeralRunnerSetIntegrityHash := runnerSet.Annotations[AnnotationKeyIntegrityHash] + originalRunnerSetUID := runnerSet.UID patched := autoscalingRunnerSet.DeepCopy() patched.Spec.GitHubConfigSecret = updatedSecret.Name @@ -838,9 +757,8 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { current := new(v1alpha1.EphemeralRunnerSet) err := k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingRunnerSet.Name, Namespace: autoscalingRunnerSet.Namespace}, current) g.Expect(err).NotTo(HaveOccurred(), "failed to get EphemeralRunnerSet") - g.Expect(current.UID).To(Equal(originalEphemeralRunnerSetUID), "EphemeralRunnerSet should be updated in place") + g.Expect(current.UID).To(Equal(originalRunnerSetUID), "EphemeralRunnerSet should be updated in place") g.Expect(current.Spec.EphemeralRunnerSpec.GitHubConfigSecret).To(Equal(updatedSecret.Name)) - g.Expect(current.Annotations[AnnotationKeyIntegrityHash]).To(Equal(originalEphemeralRunnerSetIntegrityHash), "EphemeralRunnerSet hash integrity key should not be modified") }, autoscalingRunnerSetTestTimeout, autoscalingRunnerSetTestInterval, @@ -851,9 +769,8 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { current := new(v1alpha1.AutoscalingListener) err := k8sClient.Get(ctx, client.ObjectKey{Name: scaleSetListenerName(autoscalingRunnerSet), Namespace: autoscalingRunnerSet.Namespace}, current) g.Expect(err).NotTo(HaveOccurred(), "failed to get Listener") - g.Expect(current.UID).To(Equal(originalListenerUID), "Listener should be updated in place") - g.Expect(updatedSecret.Name).To(Equal(current.Spec.GitHubConfigSecret)) - g.Expect(current.Annotations[AnnotationKeyIntegrityHash]).To(Equal(originalListenerIntegrityHash), "Listener hash integrity key should not be modified") + g.Expect(current.UID).NotTo(Equal(originalListenerUID), "Listener should be recreated") + g.Expect(current.Spec.GitHubConfigSecret).To(Equal(updatedSecret.Name)) }, autoscalingRunnerSetTestTimeout, autoscalingRunnerSetTestInterval, @@ -920,64 +837,97 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { }) }) - It("Should update Status on EphemeralRunnerSet status Update", func() { - ars := new(v1alpha1.AutoscalingRunnerSet) - Eventually( - func() (bool, error) { - err := k8sClient.Get( - ctx, - client.ObjectKey{ - Name: autoscalingRunnerSet.Name, - Namespace: autoscalingRunnerSet.Namespace, + Context("When updating an AutoscalingRunnerSet with running or pending jobs", func() { + It("It should wait for running and pending jobs to finish before applying the update.", func() { + // Wait till the listener is created + listener := new(v1alpha1.AutoscalingListener) + Eventually( + func() error { + return k8sClient.Get(ctx, client.ObjectKey{Name: scaleSetListenerName(autoscalingRunnerSet), Namespace: autoscalingRunnerSet.Namespace}, listener) + }, + autoscalingRunnerSetTestTimeout, + autoscalingRunnerSetTestInterval, + ).Should(Succeed(), "Listener should be created") + + // Wait till the ephemeral runner set is created + Eventually( + func() (int, error) { + runnerSetList := new(v1alpha1.EphemeralRunnerSetList) + err := k8sClient.List(ctx, runnerSetList, client.InNamespace(autoscalingRunnerSet.Namespace)) + if err != nil { + return 0, err + } + + return len(runnerSetList.Items), nil + }, + autoscalingRunnerSetTestTimeout, + autoscalingRunnerSetTestInterval, + ).Should(BeEquivalentTo(1), "Only one EphemeralRunnerSet should be created") + + runnerSetList := new(v1alpha1.EphemeralRunnerSetList) + err := k8sClient.List(ctx, runnerSetList, client.InNamespace(autoscalingRunnerSet.Namespace)) + Expect(err).NotTo(HaveOccurred(), "failed to list EphemeralRunnerSet") + + // Emulate running and pending jobs + runnerSet := runnerSetList.Items[0] + activeRunnerSet := runnerSet.DeepCopy() + for _, phase := range []v1alpha1.EphemeralRunnerPhase{ + v1alpha1.EphemeralRunnerPhaseRunning, + v1alpha1.EphemeralRunnerPhasePending, + } { + runner, err := controller.newEphemeralRunner(activeRunnerSet) + Expect(err).NotTo(HaveOccurred(), "Failed to create active runner") + err = k8sClient.Create(ctx, runner) + Expect(err).NotTo(HaveOccurred(), "Failed to create active runner") + + updatedRunner := runner.DeepCopy() + updatedRunner.Status.Phase = phase + err = k8sClient.Status().Patch(ctx, updatedRunner, client.MergeFrom(runner)) + Expect(err).NotTo(HaveOccurred(), "Failed to patch active runner status") + } + + // Patch the AutoScalingRunnerSet image which should trigger + // the recreation of the Listener and EphemeralRunnerSet + patched := autoscalingRunnerSet.DeepCopy() + if patched.Annotations == nil { + patched.Annotations = make(map[string]string) + } + patched.Annotations[annotationKeyIntegrityHash] = "testgroup2" + patched.Spec.Template.Spec = corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "runner", + Image: "ghcr.io/actions/abcd:1.1.1", }, - ars, - ) - if err != nil { - return false, err - } - return true, nil - }, - autoscalingRunnerSetTestTimeout, - autoscalingRunnerSetTestInterval, - ).Should(BeTrue(), "AutoscalingRunnerSet should be created") + }, + } + err = k8sClient.Patch(ctx, patched, client.MergeFrom(autoscalingRunnerSet)) + Expect(err).NotTo(HaveOccurred(), "failed to patch AutoScalingRunnerSet") + autoscalingRunnerSet = patched.DeepCopy() - runnerSetList := new(v1alpha1.EphemeralRunnerSetList) - Eventually( - func() (int, error) { - err := k8sClient.List(ctx, runnerSetList, client.InNamespace(ars.Namespace)) - if err != nil { - return 0, err - } - return len(runnerSetList.Items), nil - }, - autoscalingRunnerSetTestTimeout, - autoscalingRunnerSetTestInterval, - ).Should(BeEquivalentTo(1), "Failed to fetch runner set list") + // The EphemeralRunnerSet should not be recreated + Consistently( + func() (string, error) { + runnerSetList := new(v1alpha1.EphemeralRunnerSetList) + err := k8sClient.List(ctx, runnerSetList, client.InNamespace(autoscalingRunnerSet.Namespace)) + Expect(err).NotTo(HaveOccurred(), "failed to fetch AutoScalingRunnerSet") + return runnerSetList.Items[0].Name, nil + }, + autoscalingRunnerSetTestTimeout, + autoscalingRunnerSetTestInterval, + ).Should(Equal(activeRunnerSet.Name), "The EphemeralRunnerSet should not be recreated") - runnerSet := runnerSetList.Items[0] - statusUpdate := runnerSet.DeepCopy() - statusUpdate.Status.Phase = v1alpha1.EphemeralRunnerSetPhaseRunning - - desiredStatus := v1alpha1.AutoscalingRunnerSetStatus{ - Phase: v1alpha1.AutoscalingRunnerSetPhaseRunning, - } - - err := k8sClient.Status().Patch(ctx, statusUpdate, client.MergeFrom(&runnerSet)) - Expect(err).NotTo(HaveOccurred(), "Failed to patch runner set status") - - Eventually( - func() (v1alpha1.AutoscalingRunnerSetStatus, error) { - updated := new(v1alpha1.AutoscalingRunnerSet) - err := k8sClient.Get(ctx, client.ObjectKey{Name: autoscalingRunnerSet.Name, Namespace: autoscalingRunnerSet.Namespace}, updated) - if err != nil { - return v1alpha1.AutoscalingRunnerSetStatus{}, fmt.Errorf("failed to get AutoScalingRunnerSet: %w", err) - } - return updated.Status, nil - }, - autoscalingRunnerSetTestTimeout, - autoscalingRunnerSetTestInterval, - ).Should(BeEquivalentTo(desiredStatus), "AutoScalingRunnerSet status should be updated") + // The listener should not be recreated + Consistently( + func() error { + return k8sClient.Get(ctx, client.ObjectKey{Name: scaleSetListenerName(autoscalingRunnerSet), Namespace: autoscalingRunnerSet.Namespace}, listener) + }, + autoscalingRunnerSetTestTimeout, + autoscalingRunnerSetTestInterval, + ).ShouldNot(Succeed(), "Listener should not be recreated") + }) }) + }) var _ = Describe("Test AutoScalingController updates", Ordered, func() { @@ -1182,11 +1132,10 @@ var _ = Describe("Test AutoscalingController creation failures", Ordered, func() }, }, Spec: v1alpha1.AutoscalingRunnerSetSpec{ - GitHubConfigUrl: "https://github.com/owner/repo", - GitHubConfigSecret: "secret1", - MaxRunners: &max, - MinRunners: &min, - RunnerGroup: "testgroup", + GitHubConfigUrl: "https://github.com/owner/repo", + MaxRunners: &max, + MinRunners: &min, + RunnerGroup: "testgroup", Template: corev1.PodTemplateSpec{ Spec: corev1.PodSpec{ Containers: []corev1.Container{ @@ -1220,9 +1169,8 @@ var _ = Describe("Test AutoscalingController creation failures", Ordered, func() autoscalingRunnerSetTestInterval, ).Should(BeEquivalentTo(autoscalingRunnerSetFinalizerName), "AutoScalingRunnerSet should have a finalizer") - updated := ars.DeepCopy() - updated.Annotations = make(map[string]string) - err = k8sClient.Patch(ctx, updated, client.MergeFrom(ars)) + ars.Annotations = make(map[string]string) + err = k8sClient.Update(ctx, ars) Expect(err).NotTo(HaveOccurred(), "Update autoscaling runner set without annotation should be successful") Eventually( @@ -1326,7 +1274,7 @@ var _ = Describe("Test client optional configuration", Ordered, func() { RunnerGroup: "testgroup", Proxy: &v1alpha1.ProxyConfig{ HTTP: &v1alpha1.ProxyServerConfig{ - URL: proxy.URL, + Url: proxy.URL, }, }, Template: corev1.PodTemplateSpec{ @@ -1404,7 +1352,7 @@ var _ = Describe("Test client optional configuration", Ordered, func() { RunnerGroup: "testgroup", Proxy: &v1alpha1.ProxyConfig{ HTTP: &v1alpha1.ProxyServerConfig{ - URL: "http://test:password@" + proxy.Listener.Addr().String(), + Url: "http://test:password@" + proxy.Listener.Addr().String(), CredentialSecretRef: "proxy-credentials", }, }, diff --git a/controllers/actions.github.com/ephemeralrunner_controller_test.go b/controllers/actions.github.com/ephemeralrunner_controller_test.go index 506355ac..80c27134 100644 --- a/controllers/actions.github.com/ephemeralrunner_controller_test.go +++ b/controllers/actions.github.com/ephemeralrunner_controller_test.go @@ -223,9 +223,8 @@ var _ = Describe("EphemeralRunner", func() { ).Should(Succeed(), "failed to get ephemeral runner") // update job id to simulate job assigned - updatedER := er.DeepCopy() - updatedER.Status.JobID = "1" - err := k8sClient.Status().Patch(ctx, updatedER, client.MergeFrom(er)) + er.Status.JobID = "1" + err := k8sClient.Status().Update(ctx, er) Expect(err).To(BeNil(), "failed to update ephemeral runner status") er = new(v1alpha1.EphemeralRunner) @@ -250,8 +249,7 @@ var _ = Describe("EphemeralRunner", func() { }).Should(BeEquivalentTo(true)) // delete pod to simulate failure - updatedPod := pod.DeepCopy() - updatedPod.Status.ContainerStatuses = append(updatedPod.Status.ContainerStatuses, corev1.ContainerStatus{ + pod.Status.ContainerStatuses = append(pod.Status.ContainerStatuses, corev1.ContainerStatus{ Name: v1alpha1.EphemeralRunnerContainerName, State: corev1.ContainerState{ Terminated: &corev1.ContainerStateTerminated{ @@ -259,7 +257,7 @@ var _ = Describe("EphemeralRunner", func() { }, }, }) - err = k8sClient.Status().Patch(ctx, updatedPod, client.MergeFrom(pod)) + err = k8sClient.Status().Update(ctx, pod) Expect(err).To(BeNil(), "Failed to update pod status") er = new(v1alpha1.EphemeralRunner) @@ -279,9 +277,8 @@ var _ = Describe("EphemeralRunner", func() { return k8sClient.Get(ctx, client.ObjectKey{Name: ephemeralRunner.Name, Namespace: ephemeralRunner.Namespace}, er) }, ephemeralRunnerTimeout, ephemeralRunnerInterval).Should(Succeed(), "failed to get ephemeral runner") - updatedER := er.DeepCopy() - updatedER.Status.JobID = "1" - err := k8sClient.Status().Patch(ctx, updatedER, client.MergeFrom(er)) + er.Status.JobID = "1" + err := k8sClient.Status().Update(ctx, er) Expect(err).To(BeNil(), "failed to update ephemeral runner status") Eventually(func() (string, error) { @@ -300,10 +297,9 @@ var _ = Describe("EphemeralRunner", func() { return true, nil }, ephemeralRunnerTimeout, ephemeralRunnerInterval).Should(BeEquivalentTo(true)) - updatedPod := pod.DeepCopy() - updatedPod.Status.Phase = corev1.PodFailed - updatedPod.Status.ContainerStatuses = nil - err = k8sClient.Status().Patch(ctx, updatedPod, client.MergeFrom(pod)) + pod.Status.Phase = corev1.PodFailed + pod.Status.ContainerStatuses = nil + err = k8sClient.Status().Update(ctx, pod) Expect(err).To(BeNil(), "Failed to update pod status") Eventually(func() bool { @@ -324,10 +320,9 @@ var _ = Describe("EphemeralRunner", func() { oldPodUID := pod.UID - updatedPod := pod.DeepCopy() - updatedPod.Status.Phase = corev1.PodFailed - updatedPod.Status.ContainerStatuses = nil - err := k8sClient.Status().Patch(ctx, updatedPod, client.MergeFrom(pod)) + pod.Status.Phase = corev1.PodFailed + pod.Status.ContainerStatuses = nil + err := k8sClient.Status().Update(ctx, pod) Expect(err).To(BeNil(), "Failed to update pod status") Eventually( @@ -374,9 +369,8 @@ var _ = Describe("EphemeralRunner", func() { // Simulate init container failure without PodFailed phase. // This can happen when the kubelet has not yet transitioned the pod phase. - updatedPod := pod.DeepCopy() - updatedPod.Status.Phase = corev1.PodPending - updatedPod.Status.InitContainerStatuses = []corev1.ContainerStatus{ + pod.Status.Phase = corev1.PodPending + pod.Status.InitContainerStatuses = []corev1.ContainerStatus{ { Name: "setup", State: corev1.ContainerState{ @@ -388,7 +382,7 @@ var _ = Describe("EphemeralRunner", func() { }, }, } - err := k8sClient.Status().Patch(ctx, updatedPod, client.MergeFrom(pod)) + err := k8sClient.Status().Update(ctx, pod) Expect(err).To(BeNil(), "Failed to update pod status") Eventually( @@ -428,9 +422,8 @@ var _ = Describe("EphemeralRunner", func() { return k8sClient.Get(ctx, client.ObjectKey{Name: ephemeralRunner.Name, Namespace: ephemeralRunner.Namespace}, er) }, ephemeralRunnerTimeout, ephemeralRunnerInterval).Should(Succeed(), "failed to get ephemeral runner") - updatedER := er.DeepCopy() - updatedER.Status.JobID = "1" - err := k8sClient.Status().Patch(ctx, updatedER, client.MergeFrom(er)) + er.Status.JobID = "1" + err := k8sClient.Status().Update(ctx, er) Expect(err).To(BeNil(), "failed to update ephemeral runner status") Eventually(func() (string, error) { @@ -450,9 +443,8 @@ var _ = Describe("EphemeralRunner", func() { }, ephemeralRunnerTimeout, ephemeralRunnerInterval).Should(BeEquivalentTo(true)) // Simulate init container failure with job assigned - updatedPod := pod.DeepCopy() - updatedPod.Status.Phase = corev1.PodPending - updatedPod.Status.InitContainerStatuses = []corev1.ContainerStatus{ + pod.Status.Phase = corev1.PodPending + pod.Status.InitContainerStatuses = []corev1.ContainerStatus{ { Name: "setup", State: corev1.ContainerState{ @@ -463,7 +455,7 @@ var _ = Describe("EphemeralRunner", func() { }, }, } - err = k8sClient.Status().Patch(ctx, updatedPod, client.MergeFrom(pod)) + err = k8sClient.Status().Update(ctx, pod) Expect(err).To(BeNil(), "Failed to update pod status") Eventually(func() bool { @@ -479,9 +471,8 @@ var _ = Describe("EphemeralRunner", func() { return k8sClient.Get(ctx, client.ObjectKey{Name: ephemeralRunner.Name, Namespace: ephemeralRunner.Namespace}, er) }, ephemeralRunnerTimeout, ephemeralRunnerInterval).Should(Succeed(), "failed to get ephemeral runner") - updatedER := er.DeepCopy() - updatedER.Status.JobID = "1" - err := k8sClient.Status().Patch(ctx, updatedER, client.MergeFrom(er)) + er.Status.JobID = "1" + err := k8sClient.Status().Update(ctx, er) Expect(err).To(BeNil(), "failed to update ephemeral runner status") pod := new(corev1.Pod) @@ -496,9 +487,8 @@ var _ = Describe("EphemeralRunner", func() { ephemeralRunnerInterval, ).Should(Succeed(), "failed to get pod") - updatedPod := pod.DeepCopy() - updatedPod.Status.Phase = corev1.PodFailed - updatedPod.Status.ContainerStatuses = append(updatedPod.Status.ContainerStatuses, corev1.ContainerStatus{ + pod.Status.Phase = corev1.PodFailed + pod.Status.ContainerStatuses = append(pod.Status.ContainerStatuses, corev1.ContainerStatus{ Name: v1alpha1.EphemeralRunnerContainerName, State: corev1.ContainerState{ Terminated: &corev1.ContainerStateTerminated{ @@ -506,7 +496,7 @@ var _ = Describe("EphemeralRunner", func() { }, }, }) - err = k8sClient.Status().Patch(ctx, updatedPod, client.MergeFrom(pod)) + err = k8sClient.Status().Update(ctx, pod) Expect(err).To(BeNil(), "Failed to update pod status") Eventually( @@ -533,9 +523,8 @@ var _ = Describe("EphemeralRunner", func() { ephemeralRunnerInterval, ).Should(Succeed(), "failed to get pod") - updatedPod := pod.DeepCopy() - updatedPod.Status.Phase = corev1.PodFailed - updatedPod.Status.ContainerStatuses = append(updatedPod.Status.ContainerStatuses, corev1.ContainerStatus{ + pod.Status.Phase = corev1.PodFailed + pod.Status.ContainerStatuses = append(pod.Status.ContainerStatuses, corev1.ContainerStatus{ Name: v1alpha1.EphemeralRunnerContainerName, State: corev1.ContainerState{ Terminated: &corev1.ContainerStateTerminated{ @@ -543,7 +532,7 @@ var _ = Describe("EphemeralRunner", func() { }, }, }) - err := k8sClient.Status().Patch(ctx, updatedPod, client.MergeFrom(pod)) + err := k8sClient.Status().Update(ctx, pod) Expect(err).To(BeNil(), "Failed to update pod status") Eventually( @@ -579,10 +568,9 @@ var _ = Describe("EphemeralRunner", func() { ephemeralRunnerInterval, ).Should(Succeed(), "failed to get pod") - updatedPod := pod.DeepCopy() - updatedPod.Status.Phase = corev1.PodFailed + pod.Status.Phase = corev1.PodFailed oldPodUID := pod.UID - updatedPod.Status.ContainerStatuses = append(updatedPod.Status.ContainerStatuses, corev1.ContainerStatus{ + pod.Status.ContainerStatuses = append(pod.Status.ContainerStatuses, corev1.ContainerStatus{ Name: v1alpha1.EphemeralRunnerContainerName, State: corev1.ContainerState{ Terminated: &corev1.ContainerStateTerminated{ @@ -591,7 +579,7 @@ var _ = Describe("EphemeralRunner", func() { }, }) - err := k8sClient.Status().Patch(ctx, updatedPod, client.MergeFrom(pod)) + err := k8sClient.Status().Update(ctx, pod) Expect(err).To(BeNil(), "Failed to update pod status") Eventually( @@ -951,9 +939,8 @@ var _ = Describe("EphemeralRunner", func() { ephemeralRunnerInterval, ).Should(BeEquivalentTo(true)) - updatedPod := pod.DeepCopy() - updatedPod.Status.Phase = corev1.PodRunning - err := k8sClient.Status().Patch(ctx, updatedPod, client.MergeFrom(pod)) + pod.Status.Phase = corev1.PodRunning + err := k8sClient.Status().Update(ctx, pod) Expect(err).To(BeNil(), "failed to patch pod status") Consistently( @@ -988,8 +975,7 @@ var _ = Describe("EphemeralRunner", func() { ephemeralRunnerInterval, ).Should(Succeed(), "failed to get ephemeral runner pod") - updatedPod := pod.DeepCopy() - updatedPod.Status.ContainerStatuses = append(updatedPod.Status.ContainerStatuses, corev1.ContainerStatus{ + pod.Status.ContainerStatuses = append(pod.Status.ContainerStatuses, corev1.ContainerStatus{ Name: v1alpha1.EphemeralRunnerContainerName, State: corev1.ContainerState{ Terminated: &corev1.ContainerStateTerminated{ @@ -997,10 +983,10 @@ var _ = Describe("EphemeralRunner", func() { }, }, }) - err := k8sClient.Status().Patch(ctx, updatedPod, client.MergeFrom(pod)) + err := k8sClient.Status().Update(ctx, pod) Expect(err).To(BeNil(), "Failed to update pod status") - return updatedPod + return pod } for i := range 5 { @@ -1079,14 +1065,13 @@ var _ = Describe("EphemeralRunner", func() { ephemeralRunnerInterval, ).Should(BeEquivalentTo(true)) - updatedPod := pod.DeepCopy() - updatedPod.Status.Phase = corev1.PodFailed - updatedPod.Status.Reason = "Evicted" - updatedPod.Status.ContainerStatuses = append(updatedPod.Status.ContainerStatuses, corev1.ContainerStatus{ + pod.Status.Phase = corev1.PodFailed + pod.Status.Reason = "Evicted" + pod.Status.ContainerStatuses = append(pod.Status.ContainerStatuses, corev1.ContainerStatus{ Name: v1alpha1.EphemeralRunnerContainerName, State: corev1.ContainerState{}, }) - err := k8sClient.Status().Patch(ctx, updatedPod, client.MergeFrom(pod)) + err := k8sClient.Status().Update(ctx, pod) Expect(err).To(BeNil(), "failed to patch pod status") updated := new(v1alpha1.EphemeralRunner) @@ -1126,14 +1111,13 @@ var _ = Describe("EphemeralRunner", func() { ephemeralRunnerInterval, ).Should(BeEquivalentTo(true)) - updatedPod := pod.DeepCopy() - updatedPod.Status.Phase = corev1.PodFailed - updatedPod.Status.Reason = "OutOfpods" - updatedPod.Status.ContainerStatuses = append(updatedPod.Status.ContainerStatuses, corev1.ContainerStatus{ + pod.Status.Phase = corev1.PodFailed + pod.Status.Reason = "OutOfpods" + pod.Status.ContainerStatuses = append(pod.Status.ContainerStatuses, corev1.ContainerStatus{ Name: v1alpha1.EphemeralRunnerContainerName, State: corev1.ContainerState{}, }) - err := k8sClient.Status().Patch(ctx, updatedPod, client.MergeFrom(pod)) + err := k8sClient.Status().Update(ctx, pod) Expect(err).To(BeNil(), "failed to patch pod status") updated := new(v1alpha1.EphemeralRunner) @@ -1173,8 +1157,7 @@ var _ = Describe("EphemeralRunner", func() { ).Should(BeEquivalentTo(true)) // first set phase to running - updatedPod := pod.DeepCopy() - updatedPod.Status.ContainerStatuses = append(updatedPod.Status.ContainerStatuses, corev1.ContainerStatus{ + pod.Status.ContainerStatuses = append(pod.Status.ContainerStatuses, corev1.ContainerStatus{ Name: v1alpha1.EphemeralRunnerContainerName, State: corev1.ContainerState{ Running: &corev1.ContainerStateRunning{ @@ -1182,8 +1165,8 @@ var _ = Describe("EphemeralRunner", func() { }, }, }) - updatedPod.Status.Phase = corev1.PodRunning - err := k8sClient.Status().Patch(ctx, updatedPod, client.MergeFrom(pod)) + pod.Status.Phase = corev1.PodRunning + err := k8sClient.Status().Update(ctx, pod) Expect(err).To(BeNil()) Eventually( @@ -1199,9 +1182,8 @@ var _ = Describe("EphemeralRunner", func() { ).Should(BeEquivalentTo(v1alpha1.EphemeralRunnerPhaseRunning)) // set phase to succeeded - nextPod := updatedPod.DeepCopy() - nextPod.Status.Phase = corev1.PodSucceeded - err = k8sClient.Status().Patch(ctx, nextPod, client.MergeFrom(updatedPod)) + pod.Status.Phase = corev1.PodSucceeded + err = k8sClient.Status().Update(ctx, pod) Expect(err).To(BeNil()) Consistently( @@ -1276,8 +1258,7 @@ var _ = Describe("EphemeralRunner", func() { return true, nil }, ephemeralRunnerTimeout, ephemeralRunnerInterval).Should(BeEquivalentTo(true)) - updatedPod := pod.DeepCopy() - updatedPod.Status.ContainerStatuses = append(updatedPod.Status.ContainerStatuses, corev1.ContainerStatus{ + pod.Status.ContainerStatuses = append(pod.Status.ContainerStatuses, corev1.ContainerStatus{ Name: v1alpha1.EphemeralRunnerContainerName, State: corev1.ContainerState{ Terminated: &corev1.ContainerStateTerminated{ @@ -1285,7 +1266,7 @@ var _ = Describe("EphemeralRunner", func() { }, }, }) - err = k8sClient.Status().Patch(ctx, updatedPod, client.MergeFrom(pod)) + err = k8sClient.Status().Update(ctx, pod) Expect(err).To(BeNil(), "failed to update pod status") updated := new(v1alpha1.EphemeralRunner) @@ -1386,7 +1367,7 @@ var _ = Describe("EphemeralRunner", func() { ephemeralRunner.Spec.GitHubConfigURL = "http://example.com/org/repo" ephemeralRunner.Spec.Proxy = &v1alpha1.ProxyConfig{ HTTP: &v1alpha1.ProxyServerConfig{ - URL: proxy.URL, + Url: proxy.URL, CredentialSecretRef: "proxy-credentials", }, } @@ -1407,10 +1388,10 @@ var _ = Describe("EphemeralRunner", func() { ephemeralRunner := newExampleRunner("test-runner", autoScalingNS.Name, configSecret.Name) ephemeralRunner.Spec.Proxy = &v1alpha1.ProxyConfig{ HTTP: &v1alpha1.ProxyServerConfig{ - URL: "http://proxy.example.com:8080", + Url: "http://proxy.example.com:8080", }, HTTPS: &v1alpha1.ProxyServerConfig{ - URL: "http://proxy.example.com:8080", + Url: "http://proxy.example.com:8080", }, NoProxy: []string{"example.com"}, } diff --git a/controllers/actions.github.com/ephemeralrunnerset_controller_test.go b/controllers/actions.github.com/ephemeralrunnerset_controller_test.go index 8d80ce0e..3c391024 100644 --- a/controllers/actions.github.com/ephemeralrunnerset_controller_test.go +++ b/controllers/actions.github.com/ephemeralrunnerset_controller_test.go @@ -222,25 +222,10 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { ephemeralRunnerSetTestInterval, ).Should(BeEquivalentTo(0), "No EphemeralRunner should be created") - // Check if the status is initialized - Consistently( - func() (v1alpha1.EphemeralRunnerSetPhase, error) { - runnerSet := new(v1alpha1.EphemeralRunnerSet) - err := k8sClient.Get(ctx, client.ObjectKey{Name: ephemeralRunnerSet.Name, Namespace: ephemeralRunnerSet.Namespace}, runnerSet) - if err != nil { - return "", err - } - - return runnerSet.Status.Phase, nil - }, - ephemeralRunnerSetTestTimeout, - ephemeralRunnerSetTestInterval, - ).Should(BeEquivalentTo(v1alpha1.EphemeralRunnerSetPhaseRunning), "EphemeralRunnerSet status should be running") - // Scaling up the EphemeralRunnerSet updated := created.DeepCopy() updated.Spec.Replicas = 5 - err := k8sClient.Patch(ctx, updated, client.MergeFrom(created)) + err := k8sClient.Update(ctx, updated) Expect(err).NotTo(HaveOccurred(), "failed to update EphemeralRunnerSet") // Check if the number of ephemeral runners are created @@ -275,21 +260,6 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { ephemeralRunnerSetTestTimeout, ephemeralRunnerSetTestInterval, ).Should(BeEquivalentTo(5), "5 EphemeralRunner should be created") - - // Check if the status stays running - Eventually( - func() (v1alpha1.EphemeralRunnerSetPhase, error) { - runnerSet := new(v1alpha1.EphemeralRunnerSet) - err := k8sClient.Get(ctx, client.ObjectKey{Name: ephemeralRunnerSet.Name, Namespace: ephemeralRunnerSet.Namespace}, runnerSet) - if err != nil { - return "", err - } - - return runnerSet.Status.Phase, nil - }, - ephemeralRunnerSetTestTimeout, - ephemeralRunnerSetTestInterval, - ).Should(BeEquivalentTo(v1alpha1.EphemeralRunnerSetPhaseRunning), "EphemeralRunnerSet status should be running") }) }) @@ -302,7 +272,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { // Scale up the EphemeralRunnerSet updated := created.DeepCopy() updated.Spec.Replicas = 5 - err = k8sClient.Patch(ctx, updated, client.MergeFrom(created)) + err = k8sClient.Update(ctx, updated) Expect(err).NotTo(HaveOccurred(), "failed to update EphemeralRunnerSet") // Wait for the EphemeralRunnerSet to be scaled up @@ -1190,7 +1160,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { // Scale up the EphemeralRunnerSet updated := created.DeepCopy() updated.Spec.Replicas = 3 - err := k8sClient.Patch(ctx, updated, client.MergeFrom(created)) + err := k8sClient.Update(ctx, updated) Expect(err).NotTo(HaveOccurred(), "failed to update EphemeralRunnerSet replica count") runnerList := new(v1alpha1.EphemeralRunnerList) @@ -1546,11 +1516,11 @@ var _ = Describe("Test EphemeralRunnerSet controller with proxy settings", func( RunnerScaleSetID: 100, Proxy: &v1alpha1.ProxyConfig{ HTTP: &v1alpha1.ProxyServerConfig{ - URL: "http://proxy.example.com", + Url: "http://proxy.example.com", CredentialSecretRef: secretCredentials.Name, }, HTTPS: &v1alpha1.ProxyServerConfig{ - URL: "https://proxy.example.com", + Url: "https://proxy.example.com", CredentialSecretRef: secretCredentials.Name, }, NoProxy: []string{"example.com", "example.org"}, @@ -1729,7 +1699,7 @@ var _ = Describe("Test EphemeralRunnerSet controller with proxy settings", func( RunnerScaleSetID: 100, Proxy: &v1alpha1.ProxyConfig{ HTTP: &v1alpha1.ProxyServerConfig{ - URL: proxy.URL, + Url: proxy.URL, CredentialSecretRef: "proxy-credentials", }, }, diff --git a/controllers/actions.github.com/resourcebuilder.go b/controllers/actions.github.com/resourcebuilder.go index 68ce5cbc..fd456b56 100644 --- a/controllers/actions.github.com/resourcebuilder.go +++ b/controllers/actions.github.com/resourcebuilder.go @@ -46,14 +46,14 @@ var commonLabelKeys = [...]string{ LabelKeyGitHubRepository, } -// AnnotationKeyIntegrityHash is used as a hash of the important fields +// annotationKeyIntegrityHash is used as a hash of the important fields // of each resource to determine if more drastic action should be taken. // // For example, annotations/labels are not something that should modify // the behavior of a resource, while the change in spec is. Therefore, // the spec hash should contain the spec fields in order to determine // modifications. -const AnnotationKeyIntegrityHash = "actions.github.com/integrity-hash" +const annotationKeyIntegrityHash = "actions.github.com/integrity-hash" const labelValueKubernetesPartOf = "gha-runner-scale-set" @@ -115,10 +115,6 @@ func (b *ResourceBuilder) setControllerReference(owner client.Object, object cli return ctrl.SetControllerReference(owner, object, b.Scheme) } -func autoscalingRunnerSetIntegrityHash(ars *v1alpha1.AutoscalingRunnerSet) string { - return hash.ComputeTemplateHash(&ars.Spec) -} - func (b *ResourceBuilder) newAutoscalingListener(autoscalingRunnerSet *v1alpha1.AutoscalingRunnerSet, ephemeralRunnerSet *v1alpha1.EphemeralRunnerSet, namespace, image string, imagePullSecrets []corev1.LocalObjectReference) (*v1alpha1.AutoscalingListener, error) { runnerScaleSetID, err := strconv.Atoi(autoscalingRunnerSet.Annotations[runnerScaleSetIDAnnotationKey]) if err != nil { @@ -126,14 +122,13 @@ func (b *ResourceBuilder) newAutoscalingListener(autoscalingRunnerSet *v1alpha1. } effectiveMinRunners := 0 - if autoscalingRunnerSet.Spec.MinRunners != nil { - effectiveMinRunners = *autoscalingRunnerSet.Spec.MinRunners - } - effectiveMaxRunners := math.MaxInt32 if autoscalingRunnerSet.Spec.MaxRunners != nil { effectiveMaxRunners = *autoscalingRunnerSet.Spec.MaxRunners } + if autoscalingRunnerSet.Spec.MinRunners != nil { + effectiveMinRunners = *autoscalingRunnerSet.Spec.MinRunners + } spec := v1alpha1.AutoscalingListenerSpec{ GitHubConfigURL: autoscalingRunnerSet.Spec.GitHubConfigUrl, @@ -170,12 +165,12 @@ func (b *ResourceBuilder) newAutoscalingListener(autoscalingRunnerSet *v1alpha1. } annotations := map[string]string{ - AnnotationKeyIntegrityHash: spec.Hash(), + annotationKeyIntegrityHash: spec.Hash(), } if autoscalingRunnerSet.Spec.AutoscalingListenerMetadata != nil { labels = b.filterAndMergeLabels(autoscalingRunnerSet.Spec.AutoscalingListenerMetadata.Labels, labels) - annotations = b.filterAndMergeAnnotations(autoscalingRunnerSet.Spec.AutoscalingListenerMetadata.Annotations, annotations) + annotations = b.mergeAnnotations(autoscalingRunnerSet.Spec.AutoscalingListenerMetadata.Annotations, annotations) } autoscalingListener := &v1alpha1.AutoscalingListener{ @@ -283,7 +278,7 @@ func (b *ResourceBuilder) newScaleSetListenerConfig(autoscalingListener *v1alpha }, } - desiredSecret.Annotations[AnnotationKeyIntegrityHash] = scaleSetListenerConfigIntegrityHash(desiredSecret) + desiredSecret.Annotations[annotationKeyIntegrityHash] = scaleSetListenerConfigIntegrityHash(desiredSecret) if err := b.setControllerReference(autoscalingListener, desiredSecret); err != nil { return nil, fmt.Errorf("failed to set controller reference for listener config secret: %w", err) @@ -431,7 +426,7 @@ func (b *ResourceBuilder) newScaleSetListenerPod( Spec: podSpec, } - newRunnerScaleSetListenerPod.Annotations[AnnotationKeyIntegrityHash] = scaleSetListenerPodIntegrity( + newRunnerScaleSetListenerPod.Annotations[annotationKeyIntegrityHash] = scaleSetListenerPodIntegrity( newRunnerScaleSetListenerPod, autoscalingListener, podConfig, @@ -473,11 +468,11 @@ func scaleSetListenerPodIntegrity( d := data{ ListenerPodSpec: &pod.Spec, - AutoscalingListenerIntegrityHash: autoscalingListener.Annotations[AnnotationKeyIntegrityHash], - ConfigSecretIntegrityHash: podConfig.Annotations[AnnotationKeyIntegrityHash], - ServiceAccountIntegrityHash: serviceAccount.Annotations[AnnotationKeyIntegrityHash], - RoleIntegrityHash: role.Annotations[AnnotationKeyIntegrityHash], - RoleBindingIntegrityHash: roleBinding.Annotations[AnnotationKeyIntegrityHash], + AutoscalingListenerIntegrityHash: autoscalingListener.Annotations[annotationKeyIntegrityHash], + ConfigSecretIntegrityHash: podConfig.Annotations[annotationKeyIntegrityHash], + ServiceAccountIntegrityHash: serviceAccount.Annotations[annotationKeyIntegrityHash], + RoleIntegrityHash: role.Annotations[annotationKeyIntegrityHash], + RoleBindingIntegrityHash: roleBinding.Annotations[annotationKeyIntegrityHash], MetricsConfig: metricsConfig, } @@ -616,10 +611,10 @@ func (b *ResourceBuilder) newScaleSetListenerServiceAccount(autoscalingListener if autoscalingListener.Spec.ServiceAccountMetadata != nil { base.Labels = b.filterAndMergeLabels(autoscalingListener.Spec.ServiceAccountMetadata.Labels, base.Labels) - base.Annotations = b.filterAndMergeAnnotations(autoscalingListener.Spec.ServiceAccountMetadata.Annotations, base.Annotations) + base.Annotations = b.mergeAnnotations(autoscalingListener.Spec.ServiceAccountMetadata.Annotations, base.Annotations) } - base.Annotations[AnnotationKeyIntegrityHash] = scaleSetListenerServiceAccountIntegrityHash(base) + base.Annotations[annotationKeyIntegrityHash] = scaleSetListenerServiceAccountIntegrityHash(base) if err := b.setControllerReference(autoscalingListener, base); err != nil { return nil, fmt.Errorf("failed to set controller reference for listener service account: %w", err) @@ -655,7 +650,7 @@ func (b *ResourceBuilder) newScaleSetListenerRole(autoscalingListener *v1alpha1. annotations := make(map[string]string) if autoscalingListener.Spec.RoleMetadata != nil { labels = b.filterAndMergeLabels(autoscalingListener.Spec.RoleMetadata.Labels, labels) - annotations = b.filterAndMergeAnnotations(autoscalingListener.Spec.RoleMetadata.Annotations, nil) + annotations = b.mergeAnnotations(autoscalingListener.Spec.RoleMetadata.Annotations, nil) } newRole := &rbacv1.Role{ @@ -668,7 +663,7 @@ func (b *ResourceBuilder) newScaleSetListenerRole(autoscalingListener *v1alpha1. Rules: rulesForListenerRole([]string{autoscalingListener.Spec.EphemeralRunnerSetName}), } - newRole.Annotations[AnnotationKeyIntegrityHash] = scaleSetRoleIntegrityHash(newRole) + newRole.Annotations[annotationKeyIntegrityHash] = scaleSetRoleIntegrityHash(newRole) return newRole } @@ -723,7 +718,7 @@ func (b *ResourceBuilder) newScaleSetListenerRoleBinding(autoscalingListener *v1 Subjects: subjects, } - newRoleBinding.Annotations[AnnotationKeyIntegrityHash] = scaleSetListenerRoleBindingIntegrityHash(newRoleBinding) + newRoleBinding.Annotations[annotationKeyIntegrityHash] = scaleSetListenerRoleBindingIntegrityHash(newRoleBinding) return newRoleBinding } @@ -782,7 +777,7 @@ func (b *ResourceBuilder) newEphemeralRunnerSet(autoscalingRunnerSet *v1alpha1.A if autoscalingRunnerSet.Spec.EphemeralRunnerSetMetadata != nil { labels = b.filterAndMergeLabels(autoscalingRunnerSet.Spec.EphemeralRunnerSetMetadata.Labels, labels) - annotations = b.filterAndMergeAnnotations(autoscalingRunnerSet.Spec.EphemeralRunnerSetMetadata.Annotations, annotations) + annotations = b.mergeAnnotations(autoscalingRunnerSet.Spec.EphemeralRunnerSetMetadata.Annotations, annotations) } newEphemeralRunnerSet := &v1alpha1.EphemeralRunnerSet{ @@ -796,7 +791,7 @@ func (b *ResourceBuilder) newEphemeralRunnerSet(autoscalingRunnerSet *v1alpha1.A Spec: spec, } - newEphemeralRunnerSet.Annotations[AnnotationKeyIntegrityHash] = ephemeralRunnerSetIntegrityHash(newEphemeralRunnerSet) + newEphemeralRunnerSet.Annotations[annotationKeyIntegrityHash] = ephemeralRunnerSetIntegrityHash(newEphemeralRunnerSet) if err := b.setControllerReference(autoscalingRunnerSet, newEphemeralRunnerSet); err != nil { return nil, fmt.Errorf("failed to set controller reference for ephemeral runner set: %w", err) @@ -830,7 +825,7 @@ func (b *ResourceBuilder) newAutoscalingListenerProxySecret(autoscalingListener Data: data, } - newProxySecret.Annotations[AnnotationKeyIntegrityHash] = autoscalingListenerProxySecretIntegrityHash(newProxySecret) + newProxySecret.Annotations[annotationKeyIntegrityHash] = autoscalingListenerProxySecretIntegrityHash(newProxySecret) if err := b.setControllerReference(autoscalingListener, newProxySecret); err != nil { return nil, fmt.Errorf("failed to set controller reference for listener proxy secret: %w", err) @@ -862,7 +857,7 @@ func (b *ResourceBuilder) newEphemeralRunner(ephemeralRunnerSet *v1alpha1.Epheme if ephemeralRunnerSet.Spec.EphemeralRunnerMetadata != nil { labels = b.filterAndMergeLabels(ephemeralRunnerSet.Spec.EphemeralRunnerMetadata.Labels, labels) - annotations = b.filterAndMergeAnnotations(ephemeralRunnerSet.Spec.EphemeralRunnerMetadata.Annotations, annotations) + annotations = b.mergeAnnotations(ephemeralRunnerSet.Spec.EphemeralRunnerMetadata.Annotations, annotations) } ephemeralRunner := &v1alpha1.EphemeralRunner{ @@ -997,7 +992,7 @@ func (b *ResourceBuilder) newEphemeralRunnerSetProxySecret(ephemeralRunnerSet *v Data: data, } - runnerPodProxySecret.Annotations[AnnotationKeyIntegrityHash] = ephemeralRunnerSetProxySecretZIdentityHash(runnerPodProxySecret) + runnerPodProxySecret.Annotations[annotationKeyIntegrityHash] = ephemeralRunnerSetProxySecretZIdentityHash(runnerPodProxySecret) if err := b.setControllerReference(ephemeralRunnerSet, runnerPodProxySecret); err != nil { return nil, fmt.Errorf("failed to set controller reference for ephemeral runner set proxy secret: %w", err) @@ -1098,78 +1093,40 @@ func trimLabelValue(val string) string { return strings.Trim(val, "-_.") } -func (b *ResourceBuilder) filterLabels(k, v string) bool { - for _, prefix := range b.ExcludeLabelPropagationPrefixes { - if strings.HasPrefix(k, prefix) { - return true - } - } - return false -} - func (b *ResourceBuilder) filterAndMergeLabels(base, overwrite map[string]string) map[string]string { - return filterAndMergeMaps(base, overwrite, b.filterLabels) -} - -func filterAndMergeMaps(base, overwrite map[string]string, filter func(k, v string) bool) map[string]string { if base == nil && overwrite == nil { return nil } - var result map[string]string - if len(base) == 0 { - result = make(map[string]string) - } else { - result = maps.Clone(base) - } - if len(overwrite) > 0 { - maps.Copy(result, overwrite) - } - maps.DeleteFunc(result, filter) - return result -} -func (b *ResourceBuilder) filterAndMergeAnnotations(base, overwrite map[string]string) map[string]string { - if base == nil && overwrite == nil { - return nil - } - var result map[string]string - if len(base) == 0 { - result = make(map[string]string) - } else { - result = maps.Clone(base) + mergedLabels := make(map[string]string, len(base)) +base: + for k, v := range base { + for _, prefix := range b.ExcludeLabelPropagationPrefixes { + if strings.HasPrefix(k, prefix) { + continue base + } + } + mergedLabels[k] = v } +overwrite: for k, v := range overwrite { - if k == AnnotationKeyIntegrityHash { - continue + for _, prefix := range b.ExcludeLabelPropagationPrefixes { + if strings.HasPrefix(k, prefix) { + continue overwrite + } } - result[k] = v + mergedLabels[k] = v } - return result + return mergedLabels } -// compareAnnotations compares two maps of annotations, ignoring the integrity hash annotation. -func (b *ResourceBuilder) annotationsEqual(m1, m2 map[string]string) bool { - l1 := len(m1) - if _, ok := m1[AnnotationKeyIntegrityHash]; !ok { - l1++ +func (b *ResourceBuilder) mergeAnnotations(base, overwrite map[string]string) map[string]string { + if base == nil && overwrite == nil { + return nil } - l2 := len(m2) - if _, ok := m2[AnnotationKeyIntegrityHash]; !ok { - l2++ - } - if l1 != l2 { - return false - } - - for k, v1 := range m1 { - if k == AnnotationKeyIntegrityHash { - continue - } - if v2, ok := m2[k]; !ok || v1 != v2 { - return false - } - } - return true + base = maps.Clone(base) + maps.Copy(base, overwrite) + return base } diff --git a/controllers/actions.github.com/resourcebuilder_test.go b/controllers/actions.github.com/resourcebuilder_test.go index 206e6107..d0885117 100644 --- a/controllers/actions.github.com/resourcebuilder_test.go +++ b/controllers/actions.github.com/resourcebuilder_test.go @@ -113,7 +113,7 @@ func TestMetadataPropagation(t *testing.T) { assert.Equal(t, labelValueKubernetesPartOf, ephemeralRunnerSet.Labels[LabelKeyKubernetesPartOf]) assert.Equal(t, "runner-set", ephemeralRunnerSet.Labels[LabelKeyKubernetesComponent]) assert.Equal(t, autoscalingRunnerSet.Labels[LabelKeyKubernetesVersion], ephemeralRunnerSet.Labels[LabelKeyKubernetesVersion]) - assert.NotEmpty(t, ephemeralRunnerSet.Annotations[AnnotationKeyIntegrityHash]) + assert.NotEmpty(t, ephemeralRunnerSet.Annotations[annotationKeyIntegrityHash]) assert.Equal(t, autoscalingRunnerSet.Name, ephemeralRunnerSet.Labels[LabelKeyGitHubScaleSetName]) assert.Equal(t, autoscalingRunnerSet.Namespace, ephemeralRunnerSet.Labels[LabelKeyGitHubScaleSetNamespace]) assert.Equal(t, "", ephemeralRunnerSet.Labels[LabelKeyGitHubEnterprise]) @@ -130,7 +130,7 @@ func TestMetadataPropagation(t *testing.T) { assert.Equal(t, labelValueKubernetesPartOf, listener.Labels[LabelKeyKubernetesPartOf]) assert.Equal(t, "runner-scale-set-listener", listener.Labels[LabelKeyKubernetesComponent]) assert.Equal(t, autoscalingRunnerSet.Labels[LabelKeyKubernetesVersion], listener.Labels[LabelKeyKubernetesVersion]) - assert.NotEmpty(t, ephemeralRunnerSet.Annotations[AnnotationKeyIntegrityHash]) + assert.NotEmpty(t, ephemeralRunnerSet.Annotations[annotationKeyIntegrityHash]) assert.Equal(t, autoscalingRunnerSet.Name, listener.Labels[LabelKeyGitHubScaleSetName]) assert.Equal(t, autoscalingRunnerSet.Namespace, listener.Labels[LabelKeyGitHubScaleSetNamespace]) assert.Equal(t, "", listener.Labels[LabelKeyGitHubEnterprise]) @@ -221,7 +221,7 @@ func TestEphemeralRunnerSetProxySecretZIdentityHash(t *testing.T) { }) require.NoError(t, err) - actualHash := proxySecret.Annotations[AnnotationKeyIntegrityHash] + actualHash := proxySecret.Annotations[annotationKeyIntegrityHash] assert.NotEmpty(t, actualHash) assert.Equal(t, ephemeralRunnerSetProxySecretZIdentityHash(proxySecret), actualHash) @@ -313,7 +313,7 @@ func TestOwnershipRelationships(t *testing.T) { runnerScaleSetIDAnnotationKey: "1", AnnotationKeyGitHubRunnerGroupName: "test-group", AnnotationKeyGitHubRunnerScaleSetName: "test-scale-set", - AnnotationKeyIntegrityHash: "test-hash", + annotationKeyIntegrityHash: "test-hash", }, }, Spec: v1alpha1.AutoscalingRunnerSetSpec{ diff --git a/controllers/actions.github.com/secretresolver/secret_resolver.go b/controllers/actions.github.com/secretresolver/secret_resolver.go index b9f9d9cc..5f3e9561 100644 --- a/controllers/actions.github.com/secretresolver/secret_resolver.go +++ b/controllers/actions.github.com/secretresolver/secret_resolver.go @@ -85,9 +85,9 @@ func (sr *SecretResolver) GetActionsService(ctx context.Context, obj object.Acti } if proxy.HTTP != nil { - u, err := url.Parse(proxy.HTTP.URL) + u, err := url.Parse(proxy.HTTP.Url) if err != nil { - return nil, fmt.Errorf("failed to parse proxy http url %q: %w", proxy.HTTP.URL, err) + return nil, fmt.Errorf("failed to parse proxy http url %q: %w", proxy.HTTP.Url, err) } if ref := proxy.HTTP.CredentialSecretRef; ref != "" { @@ -101,9 +101,9 @@ func (sr *SecretResolver) GetActionsService(ctx context.Context, obj object.Acti } if proxy.HTTPS != nil { - u, err := url.Parse(proxy.HTTPS.URL) + u, err := url.Parse(proxy.HTTPS.Url) if err != nil { - return nil, fmt.Errorf("failed to parse proxy https url %q: %w", proxy.HTTPS.URL, err) + return nil, fmt.Errorf("failed to parse proxy https url %q: %w", proxy.HTTPS.Url, err) } if ref := proxy.HTTPS.CredentialSecretRef; ref != "" { diff --git a/controllers/actions.github.com/utils.go b/controllers/actions.github.com/utils.go index da87eb7b..a77b24ba 100644 --- a/controllers/actions.github.com/utils.go +++ b/controllers/actions.github.com/utils.go @@ -1,8 +1,6 @@ package actionsgithubcom import ( - "encoding/json" - "k8s.io/apimachinery/pkg/util/rand" ) @@ -27,11 +25,3 @@ func RandStringRunes(n int) string { } return string(b) } - -func mustJSON(v any) string { - val, err := json.Marshal(v) - if err != nil { - panic(err) - } - return string(val) -}