diff --git a/apis/actions.github.com/v1alpha1/autoscalinglistener_types.go b/apis/actions.github.com/v1alpha1/autoscalinglistener_types.go index 1ae599f1fb..990a6196dd 100644 --- a/apis/actions.github.com/v1alpha1/autoscalinglistener_types.go +++ b/apis/actions.github.com/v1alpha1/autoscalinglistener_types.go @@ -24,36 +24,37 @@ import ( // AutoscalingListenerSpec defines the desired state of AutoscalingListener type AutoscalingListenerSpec struct { - // Required + // +optional GitHubConfigURL string `json:"githubConfigUrl,omitempty"` - // Required + // +optional GitHubConfigSecret string `json:"githubConfigSecret,omitempty"` - // Required + // +optional + // +kubebuilder:validation:Minimum=1 RunnerScaleSetID int `json:"runnerScaleSetId,omitempty"` - // Required + // +optional AutoscalingRunnerSetNamespace string `json:"autoscalingRunnerSetNamespace,omitempty"` - // Required + // +optional AutoscalingRunnerSetName string `json:"autoscalingRunnerSetName,omitempty"` - // Required + // +optional EphemeralRunnerSetName string `json:"ephemeralRunnerSetName,omitempty"` - // Required - // +kubebuilder:validation:Minimum:=0 - MaxRunners int `json:"maxRunners,omitempty"` + // +kubebuilder:validation:Minimum=0 + // +optional + MaxRunners int `json:"maxRunners"` - // Required - // +kubebuilder:validation:Minimum:=0 - MinRunners int `json:"minRunners,omitempty"` + // +kubebuilder:validation:Minimum=0 + // +optional + MinRunners int `json:"minRunners"` - // Required + // +optional Image string `json:"image,omitempty"` - // Required + // +optional ImagePullSecrets []corev1.LocalObjectReference `json:"imagePullSecrets,omitempty"` // +optional @@ -99,17 +100,22 @@ type AutoscalingListenerStatus struct{} // AutoscalingListener is the Schema for the autoscalinglisteners API type AutoscalingListener struct { - metav1.TypeMeta `json:",inline"` + metav1.TypeMeta `json:",inline"` + // +optional metav1.ObjectMeta `json:"metadata,omitempty"` - Spec AutoscalingListenerSpec `json:"spec,omitempty"` + // +optional + Spec AutoscalingListenerSpec `json:"spec,omitempty"` + // +optional Status AutoscalingListenerStatus `json:"status,omitempty"` } +// AutoscalingListenerList is a list of AutoscalingListener resources // +kubebuilder:object:root=true // AutoscalingListenerList contains a list of AutoscalingListener type AutoscalingListenerList struct { metav1.TypeMeta `json:",inline"` + // +optional metav1.ListMeta `json:"metadata,omitempty"` Items []AutoscalingListener `json:"items"` } diff --git a/apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go b/apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go index 908e1acc9d..804efc9c60 100644 --- a/apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go +++ b/apis/actions.github.com/v1alpha1/autoscalingrunnerset_types.go @@ -40,24 +40,26 @@ import ( // +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.finishedEphemeralRunners",name=Finished Runners,type=integer -// +kubebuilder:printcolumn:JSONPath=".status.deletingEphemeralRunners",name=Deleting 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"` + metav1.TypeMeta `json:",inline"` + // +optional metav1.ObjectMeta `json:"metadata,omitempty"` - Spec AutoscalingRunnerSetSpec `json:"spec,omitempty"` + // +optional + Spec AutoscalingRunnerSetSpec `json:"spec,omitempty"` + // +optional Status AutoscalingRunnerSetStatus `json:"status,omitempty"` } // AutoscalingRunnerSetSpec defines the desired state of AutoscalingRunnerSet type AutoscalingRunnerSetSpec struct { - // Required + // +optional GitHubConfigUrl string `json:"githubConfigUrl,omitempty"` - // Required + // +optional GitHubConfigSecret string `json:"githubConfigSecret,omitempty"` // +optional @@ -78,7 +80,7 @@ type AutoscalingRunnerSetSpec struct { // +optional VaultConfig *VaultConfig `json:"vaultConfig,omitempty"` - // Required + // +optional Template corev1.PodTemplateSpec `json:"template,omitempty"` // +optional @@ -112,16 +114,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"` } @@ -158,7 +160,7 @@ func (c *TLSConfig) ToCertPool(keyFetcher func(name, key string) ([]byte, error) } type TLSCertificateSource struct { - // Required + // +required ConfigMapKeyRef *corev1.ConfigMapKeySelector `json:"configMapKeyRef,omitempty"` } @@ -179,9 +181,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 != "" { @@ -204,9 +206,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 != "" { @@ -259,8 +261,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"` @@ -315,6 +317,7 @@ type HistogramMetric struct { // AutoscalingRunnerSetStatus defines the observed state of AutoscalingRunnerSet type AutoscalingRunnerSetStatus struct { // +optional + // +kubebuilder:validation:Minimum=0 CurrentRunners int `json:"currentRunners"` // +optional @@ -323,10 +326,13 @@ type AutoscalingRunnerSetStatus struct { // EphemeralRunner counts separated by the stage ephemeral runners are in, taken from the EphemeralRunnerSet // +optional + // +kubebuilder:validation:Minimum=0 PendingEphemeralRunners int `json:"pendingEphemeralRunners"` // +optional + // +kubebuilder:validation:Minimum=0 RunningEphemeralRunners int `json:"runningEphemeralRunners"` // +optional + // +kubebuilder:validation:Minimum=0 FailedEphemeralRunners int `json:"failedEphemeralRunners"` } @@ -340,20 +346,6 @@ 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/ephemeralrunner_types.go b/apis/actions.github.com/v1alpha1/ephemeralrunner_types.go index 744afab505..e53f257af3 100644 --- a/apis/actions.github.com/v1alpha1/ephemeralrunner_types.go +++ b/apis/actions.github.com/v1alpha1/ephemeralrunner_types.go @@ -41,10 +41,13 @@ const EphemeralRunnerContainerName = "runner" // EphemeralRunner is the Schema for the ephemeralrunners API type EphemeralRunner struct { - metav1.TypeMeta `json:",inline"` + metav1.TypeMeta `json:",inline"` + // +optional metav1.ObjectMeta `json:"metadata,omitempty"` - Spec EphemeralRunnerSpec `json:"spec,omitempty"` + // +optional + Spec EphemeralRunnerSpec `json:"spec,omitempty"` + // +optional Status EphemeralRunnerStatus `json:"status,omitempty"` } @@ -102,17 +105,17 @@ func (er *EphemeralRunner) VaultProxy() *ProxyConfig { // EphemeralRunnerSpec defines the desired state of EphemeralRunner type EphemeralRunnerSpec struct { - // +required + // +optional GitHubConfigURL string `json:"githubConfigUrl,omitempty"` - // +required + // +optional GitHubConfigSecret string `json:"githubConfigSecret,omitempty"` // +optional GitHubServerTLS *TLSConfig `json:"githubServerTLS,omitempty"` - // +required - RunnerScaleSetID int `json:"runnerScaleSetId,omitempty"` + // +optional + RunnerScaleSetID int `json:"runnerScaleSetId"` // +optional Proxy *ProxyConfig `json:"proxy,omitempty"` @@ -126,6 +129,7 @@ type EphemeralRunnerSpec struct { // +optional EphemeralRunnerConfigSecretMetadata *ResourceMeta `json:"ephemeralRunnerConfigSecretMetadata,omitempty"` + // +optional corev1.PodTemplateSpec `json:",inline"` } diff --git a/apis/actions.github.com/v1alpha1/ephemeralrunnerset_types.go b/apis/actions.github.com/v1alpha1/ephemeralrunnerset_types.go index 9453f8a16e..de411ab87c 100644 --- a/apis/actions.github.com/v1alpha1/ephemeralrunnerset_types.go +++ b/apis/actions.github.com/v1alpha1/ephemeralrunnerset_types.go @@ -23,10 +23,13 @@ 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, @@ -38,12 +41,17 @@ type EphemeralRunnerSetSpec struct { // EphemeralRunnerSetStatus defines the observed state of EphemeralRunnerSet type EphemeralRunnerSetStatus struct { // CurrentReplicas is the number of currently running EphemeralRunner resources being managed by this EphemeralRunnerSet. + // +kubebuilder:validation:Minimum=0 + // +optional CurrentReplicas int `json:"currentReplicas"` // +optional + // +kubebuilder:validation:Minimum=0 PendingEphemeralRunners int `json:"pendingEphemeralRunners"` // +optional + // +kubebuilder:validation:Minimum=0 RunningEphemeralRunners int `json:"runningEphemeralRunners"` // +optional + // +kubebuilder:validation:Minimum=0 FailedEphemeralRunners int `json:"failedEphemeralRunners"` // +optional Phase EphemeralRunnerSetPhase `json:"phase"` @@ -71,10 +79,13 @@ const ( // EphemeralRunnerSet is the Schema for the ephemeralrunnersets API type EphemeralRunnerSet struct { - metav1.TypeMeta `json:",inline"` + metav1.TypeMeta `json:",inline"` + // +optional metav1.ObjectMeta `json:"metadata,omitempty"` - Spec EphemeralRunnerSetSpec `json:"spec,omitempty"` + // +optional + Spec EphemeralRunnerSetSpec `json:"spec,omitempty"` + // +optional 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 9291cde4e0..0357e5e268 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 20e57e3398..84e243f9a6 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,10 +51,8 @@ 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 @@ -70,21 +68,17 @@ 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: Required + description: Selects a key from a ConfigMap. properties: key: description: The key to select. @@ -106,13 +100,15 @@ 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 @@ -131,7 +127,6 @@ spec: x-kubernetes-map-type: atomic type: array maxRunners: - description: Required minimum: 0 type: integer metrics: @@ -183,7 +178,6 @@ spec: type: object type: object minRunners: - description: Required minimum: 0 type: integer proxy: @@ -193,16 +187,18 @@ 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: @@ -236,7 +232,7 @@ spec: type: object type: object runnerScaleSetId: - description: Required + minimum: 1 type: integer serviceAccountMetadata: description: ResourceMeta carries metadata common to all internal @@ -8782,16 +8778,18 @@ 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 d788cc1c2e..b303ff1400 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 @@ -33,11 +33,8 @@ spec: - jsonPath: .status.runningEphemeralRunners name: Running Runners type: integer - - jsonPath: .status.finishedEphemeralRunners - name: Finished Runners - type: integer - - jsonPath: .status.deletingEphemeralRunners - name: Deleting Runners + - jsonPath: .status.failedEphemeralRunners + name: Failed Runners type: integer name: v1alpha1 schema: @@ -113,18 +110,15 @@ spec: type: object type: object githubConfigSecret: - description: Required type: string githubConfigUrl: - description: Required type: string githubServerTLS: properties: certificateFrom: - description: Required properties: configMapKeyRef: - description: Required + description: Selects a key from a ConfigMap. properties: key: description: The key to select. @@ -145,7 +139,11 @@ 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 @@ -8366,16 +8364,18 @@ 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 +8391,7 @@ spec: runnerScaleSetName: type: string template: - description: Required + description: PodTemplateSpec describes the data a pod should have when created from a template properties: metadata: description: |- @@ -16517,16 +16517,18 @@ 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: @@ -16544,14 +16546,18 @@ spec: description: AutoscalingRunnerSetStatus defines the observed state of AutoscalingRunnerSet properties: currentRunners: + minimum: 0 type: integer failedEphemeralRunners: + minimum: 0 type: integer pendingEphemeralRunners: + minimum: 0 type: integer phase: type: string runningEphemeralRunners: + minimum: 0 type: integer type: object type: object 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 3cd90148ca..8248263e7d 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,10 +89,9 @@ spec: githubServerTLS: properties: certificateFrom: - description: Required properties: configMapKeyRef: - description: Required + description: Selects a key from a ConfigMap. properties: key: description: The key to select. @@ -113,7 +112,11 @@ spec: - key type: object x-kubernetes-map-type: atomic + required: + - configMapKeyRef type: object + required: + - certificateFrom type: object metadata: description: |- @@ -144,16 +147,18 @@ 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: @@ -8268,16 +8273,18 @@ 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: @@ -8290,10 +8297,6 @@ 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 7e8ade4aa6..04c274bbf2 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 @@ -98,10 +98,9 @@ spec: githubServerTLS: properties: certificateFrom: - description: Required properties: configMapKeyRef: - description: Required + description: Selects a key from a ConfigMap. properties: key: description: The key to select. @@ -122,7 +121,11 @@ spec: - key type: object x-kubernetes-map-type: atomic + required: + - configMapKeyRef type: object + required: + - certificateFrom type: object metadata: description: |- @@ -153,16 +156,18 @@ 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: @@ -8277,16 +8282,18 @@ 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: @@ -8299,10 +8306,6 @@ 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 @@ -8310,26 +8313,26 @@ 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 properties: currentReplicas: description: CurrentReplicas is the number of currently running EphemeralRunner resources being managed by this EphemeralRunnerSet. + minimum: 0 type: integer failedEphemeralRunners: + minimum: 0 type: integer pendingEphemeralRunners: + minimum: 0 type: integer phase: description: EphemeralRunnerSetPhase is the phase of the ephemeral runner set resource type: string runningEphemeralRunners: + minimum: 0 type: integer - required: - - currentReplicas type: object type: object served: true 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 20e57e3398..84e243f9a6 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,10 +51,8 @@ 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 @@ -70,21 +68,17 @@ 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: Required + description: Selects a key from a ConfigMap. properties: key: description: The key to select. @@ -106,13 +100,15 @@ 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 @@ -131,7 +127,6 @@ spec: x-kubernetes-map-type: atomic type: array maxRunners: - description: Required minimum: 0 type: integer metrics: @@ -183,7 +178,6 @@ spec: type: object type: object minRunners: - description: Required minimum: 0 type: integer proxy: @@ -193,16 +187,18 @@ 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: @@ -236,7 +232,7 @@ spec: type: object type: object runnerScaleSetId: - description: Required + minimum: 1 type: integer serviceAccountMetadata: description: ResourceMeta carries metadata common to all internal @@ -8782,16 +8778,18 @@ 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 d788cc1c2e..b303ff1400 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 @@ -33,11 +33,8 @@ spec: - jsonPath: .status.runningEphemeralRunners name: Running Runners type: integer - - jsonPath: .status.finishedEphemeralRunners - name: Finished Runners - type: integer - - jsonPath: .status.deletingEphemeralRunners - name: Deleting Runners + - jsonPath: .status.failedEphemeralRunners + name: Failed Runners type: integer name: v1alpha1 schema: @@ -113,18 +110,15 @@ spec: type: object type: object githubConfigSecret: - description: Required type: string githubConfigUrl: - description: Required type: string githubServerTLS: properties: certificateFrom: - description: Required properties: configMapKeyRef: - description: Required + description: Selects a key from a ConfigMap. properties: key: description: The key to select. @@ -145,7 +139,11 @@ 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 @@ -8366,16 +8364,18 @@ 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 +8391,7 @@ spec: runnerScaleSetName: type: string template: - description: Required + description: PodTemplateSpec describes the data a pod should have when created from a template properties: metadata: description: |- @@ -16517,16 +16517,18 @@ 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: @@ -16544,14 +16546,18 @@ spec: description: AutoscalingRunnerSetStatus defines the observed state of AutoscalingRunnerSet properties: currentRunners: + minimum: 0 type: integer failedEphemeralRunners: + minimum: 0 type: integer pendingEphemeralRunners: + minimum: 0 type: integer phase: type: string runningEphemeralRunners: + minimum: 0 type: integer type: object type: object 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 3cd90148ca..8248263e7d 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,10 +89,9 @@ spec: githubServerTLS: properties: certificateFrom: - description: Required properties: configMapKeyRef: - description: Required + description: Selects a key from a ConfigMap. properties: key: description: The key to select. @@ -113,7 +112,11 @@ spec: - key type: object x-kubernetes-map-type: atomic + required: + - configMapKeyRef type: object + required: + - certificateFrom type: object metadata: description: |- @@ -144,16 +147,18 @@ 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: @@ -8268,16 +8273,18 @@ 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: @@ -8290,10 +8297,6 @@ 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 7e8ade4aa6..04c274bbf2 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 @@ -98,10 +98,9 @@ spec: githubServerTLS: properties: certificateFrom: - description: Required properties: configMapKeyRef: - description: Required + description: Selects a key from a ConfigMap. properties: key: description: The key to select. @@ -122,7 +121,11 @@ spec: - key type: object x-kubernetes-map-type: atomic + required: + - configMapKeyRef type: object + required: + - certificateFrom type: object metadata: description: |- @@ -153,16 +156,18 @@ 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: @@ -8277,16 +8282,18 @@ 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: @@ -8299,10 +8306,6 @@ 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 @@ -8310,26 +8313,26 @@ 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 properties: currentReplicas: description: CurrentReplicas is the number of currently running EphemeralRunner resources being managed by this EphemeralRunnerSet. + minimum: 0 type: integer failedEphemeralRunners: + minimum: 0 type: integer pendingEphemeralRunners: + minimum: 0 type: integer phase: description: EphemeralRunnerSetPhase is the phase of the ephemeral runner set resource type: string runningEphemeralRunners: + minimum: 0 type: integer - required: - - currentReplicas type: object type: object served: true 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 6589d01d1c..672c8495d4 100644 --- a/charts/gha-runner-scale-set-experimental/templates/_mode_kubernetes.tpl +++ b/charts/gha-runner-scale-set-experimental/templates/_mode_kubernetes.tpl @@ -82,7 +82,10 @@ volumeMounts: subPath: extension readOnly: true {{- end }} - {{ include "githubServerTLS.volumeMountItem" (dict "root" $ "existingVolumeMounts" (list)) | nindent 2 }} + {{- with .Values.runner.container.volumeMounts }} + {{- toYaml . | nindent 2 }} + {{- end }} + {{ include "githubServerTLS.volumeMountItem" (dict "root" $ "existingVolumeMounts" (.Values.runner.container.volumeMounts | default (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 2990ccc49f..a5c9a258ca 100644 --- a/charts/gha-runner-scale-set-experimental/templates/manager_role.yaml +++ b/charts/gha-runner-scale-set-experimental/templates/manager_role.yaml @@ -17,6 +17,8 @@ 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 bbf9279999..88ca5a3f76 100644 --- a/charts/gha-runner-scale-set/templates/manager_role.yaml +++ b/charts/gha-runner-scale-set/templates/manager_role.yaml @@ -41,6 +41,8 @@ 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 ba67ea90ec..10a3ddfdfe 100644 --- a/charts/gha-runner-scale-set/tests/template_test.go +++ b/charts/gha-runner-scale-set/tests/template_test.go @@ -1364,11 +1364,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 20e57e3398..84e243f9a6 100644 --- a/config/crd/bases/actions.github.com_autoscalinglisteners.yaml +++ b/config/crd/bases/actions.github.com_autoscalinglisteners.yaml @@ -51,10 +51,8 @@ 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 @@ -70,21 +68,17 @@ 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: Required + description: Selects a key from a ConfigMap. properties: key: description: The key to select. @@ -106,13 +100,15 @@ 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 @@ -131,7 +127,6 @@ spec: x-kubernetes-map-type: atomic type: array maxRunners: - description: Required minimum: 0 type: integer metrics: @@ -183,7 +178,6 @@ spec: type: object type: object minRunners: - description: Required minimum: 0 type: integer proxy: @@ -193,16 +187,18 @@ 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: @@ -236,7 +232,7 @@ spec: type: object type: object runnerScaleSetId: - description: Required + minimum: 1 type: integer serviceAccountMetadata: description: ResourceMeta carries metadata common to all internal @@ -8782,16 +8778,18 @@ 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 d788cc1c2e..b303ff1400 100644 --- a/config/crd/bases/actions.github.com_autoscalingrunnersets.yaml +++ b/config/crd/bases/actions.github.com_autoscalingrunnersets.yaml @@ -33,11 +33,8 @@ spec: - jsonPath: .status.runningEphemeralRunners name: Running Runners type: integer - - jsonPath: .status.finishedEphemeralRunners - name: Finished Runners - type: integer - - jsonPath: .status.deletingEphemeralRunners - name: Deleting Runners + - jsonPath: .status.failedEphemeralRunners + name: Failed Runners type: integer name: v1alpha1 schema: @@ -113,18 +110,15 @@ spec: type: object type: object githubConfigSecret: - description: Required type: string githubConfigUrl: - description: Required type: string githubServerTLS: properties: certificateFrom: - description: Required properties: configMapKeyRef: - description: Required + description: Selects a key from a ConfigMap. properties: key: description: The key to select. @@ -145,7 +139,11 @@ 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 @@ -8366,16 +8364,18 @@ 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 +8391,7 @@ spec: runnerScaleSetName: type: string template: - description: Required + description: PodTemplateSpec describes the data a pod should have when created from a template properties: metadata: description: |- @@ -16517,16 +16517,18 @@ 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: @@ -16544,14 +16546,18 @@ spec: description: AutoscalingRunnerSetStatus defines the observed state of AutoscalingRunnerSet properties: currentRunners: + minimum: 0 type: integer failedEphemeralRunners: + minimum: 0 type: integer pendingEphemeralRunners: + minimum: 0 type: integer phase: type: string runningEphemeralRunners: + minimum: 0 type: integer type: object type: object diff --git a/config/crd/bases/actions.github.com_ephemeralrunners.yaml b/config/crd/bases/actions.github.com_ephemeralrunners.yaml index 3cd90148ca..8248263e7d 100644 --- a/config/crd/bases/actions.github.com_ephemeralrunners.yaml +++ b/config/crd/bases/actions.github.com_ephemeralrunners.yaml @@ -89,10 +89,9 @@ spec: githubServerTLS: properties: certificateFrom: - description: Required properties: configMapKeyRef: - description: Required + description: Selects a key from a ConfigMap. properties: key: description: The key to select. @@ -113,7 +112,11 @@ spec: - key type: object x-kubernetes-map-type: atomic + required: + - configMapKeyRef type: object + required: + - certificateFrom type: object metadata: description: |- @@ -144,16 +147,18 @@ 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: @@ -8268,16 +8273,18 @@ 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: @@ -8290,10 +8297,6 @@ 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 7e8ade4aa6..04c274bbf2 100644 --- a/config/crd/bases/actions.github.com_ephemeralrunnersets.yaml +++ b/config/crd/bases/actions.github.com_ephemeralrunnersets.yaml @@ -98,10 +98,9 @@ spec: githubServerTLS: properties: certificateFrom: - description: Required properties: configMapKeyRef: - description: Required + description: Selects a key from a ConfigMap. properties: key: description: The key to select. @@ -122,7 +121,11 @@ spec: - key type: object x-kubernetes-map-type: atomic + required: + - configMapKeyRef type: object + required: + - certificateFrom type: object metadata: description: |- @@ -153,16 +156,18 @@ 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: @@ -8277,16 +8282,18 @@ 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: @@ -8299,10 +8306,6 @@ 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 @@ -8310,26 +8313,26 @@ 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 properties: currentReplicas: description: CurrentReplicas is the number of currently running EphemeralRunner resources being managed by this EphemeralRunnerSet. + minimum: 0 type: integer failedEphemeralRunners: + minimum: 0 type: integer pendingEphemeralRunners: + minimum: 0 type: integer phase: description: EphemeralRunnerSetPhase is the phase of the ephemeral runner set resource type: string runningEphemeralRunners: + minimum: 0 type: integer - required: - - currentReplicas type: object type: object served: true diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index dc0becfdf7..007b85160e 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -52,7 +52,7 @@ rules: - delete - get - list - - update + - patch - watch - apiGroups: - actions.github.com @@ -167,5 +167,5 @@ rules: - delete - get - list - - update + - patch - watch diff --git a/controllers/actions.github.com/autoscalinglistener_controller.go b/controllers/actions.github.com/autoscalinglistener_controller.go index 7d12f895d6..a62462e9fc 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;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=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=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 @@ -170,15 +170,15 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. updatedServiceAccount.Labels = desiredLabels shouldUpdate = true } - desiredAnnotations := r.mergeAnnotations(serviceAccount.Annotations, desiredServiceAccount.Annotations) - if !maps.Equal(serviceAccount.Annotations, desiredAnnotations) { + desiredAnnotations := r.filterAndMergeAnnotations(serviceAccount.Annotations, desiredServiceAccount.Annotations) + if !r.annotationsEqual(serviceAccount.Annotations, desiredAnnotations) { updatedServiceAccount.Annotations = desiredAnnotations shouldUpdate = true } if shouldUpdate { log.Info("Updating listener service account") - if err := r.Update(ctx, updatedServiceAccount); err != nil { + if err := r.Patch(ctx, updatedServiceAccount, client.MergeFrom(&serviceAccount)); err != nil { log.Error(err, "Failed to update listener service account") return ctrl.Result{}, err } @@ -214,8 +214,8 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. updatedRole.Labels = desiredLabels shouldUpdate = true } - desiredAnnotations := r.mergeAnnotations(listenerRole.Annotations, desiredRole.Annotations) - if !maps.Equal(listenerRole.Annotations, desiredAnnotations) { + desiredAnnotations := r.filterAndMergeAnnotations(listenerRole.Annotations, desiredRole.Annotations) + if !r.annotationsEqual(listenerRole.Annotations, desiredAnnotations) { updatedRole.Annotations = desiredAnnotations shouldUpdate = true } @@ -225,7 +225,7 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. } if shouldUpdate { log.Info("Updating listener role") - if err := r.Update(ctx, updatedRole); err != nil { + if err := r.Patch(ctx, updatedRole, client.MergeFrom(&listenerRole)); err != nil { log.Error(err, "Failed to update listener role") return ctrl.Result{}, err } @@ -257,14 +257,14 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. updatedRoleBinding.Labels = desiredLabels shouldUpdate = true } - desiredAnnotations := r.mergeAnnotations(listenerRoleBinding.Annotations, desiredRoleBinding.Annotations) - if !maps.Equal(listenerRoleBinding.Annotations, desiredAnnotations) { + desiredAnnotations := r.filterAndMergeAnnotations(listenerRoleBinding.Annotations, desiredRoleBinding.Annotations) + if !r.annotationsEqual(listenerRoleBinding.Annotations, desiredAnnotations) { updatedRoleBinding.Annotations = desiredAnnotations shouldUpdate = true } if shouldUpdate { log.Info("Updating listener role binding") - if err := r.Update(ctx, updatedRoleBinding); err != nil { + if err := r.Patch(ctx, updatedRoleBinding, client.MergeFrom(&listenerRoleBinding)); err != nil { log.Error(err, "Failed to update listener role binding") return ctrl.Result{}, err } @@ -313,14 +313,14 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. updatedProxySecret.Labels = desiredLabels shouldUpdate = true } - desiredAnnotations := r.mergeAnnotations(proxySecret.Annotations, desiredListenerProxy.Annotations) - if !maps.Equal(proxySecret.Annotations, desiredAnnotations) { + desiredAnnotations := r.filterAndMergeAnnotations(proxySecret.Annotations, desiredListenerProxy.Annotations) + if !r.annotationsEqual(proxySecret.Annotations, desiredAnnotations) { updatedProxySecret.Annotations = desiredAnnotations shouldUpdate = true } if shouldUpdate { log.Info("Updating listener proxy secret") - if err := r.Update(ctx, updatedProxySecret); err != nil { + if err := r.Patch(ctx, updatedProxySecret, client.MergeFrom(&proxySecret)); err != nil { log.Error(err, "Failed to update listener proxy secret") return ctrl.Result{}, err } @@ -400,15 +400,15 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. updatedSecret.Labels = desiredLabels shouldUpdate = true } - desiredAnnotations := r.mergeAnnotations(listenerConfigSecret.Annotations, desiredSecret.Annotations) - if !maps.Equal(listenerConfigSecret.Annotations, desiredAnnotations) { + desiredAnnotations := r.filterAndMergeAnnotations(listenerConfigSecret.Annotations, desiredSecret.Annotations) + if !r.annotationsEqual(listenerConfigSecret.Annotations, desiredAnnotations) { updatedSecret.Annotations = desiredAnnotations shouldUpdate = true } if shouldUpdate { log.Info("Updating listener config secret", "namespace", updatedSecret.Namespace, "name", updatedSecret.Name) - if err := r.Update(ctx, updatedSecret); err != nil { + if err := r.Patch(ctx, updatedSecret, client.MergeFrom(&listenerConfigSecret)); err != nil { return ctrl.Result{}, fmt.Errorf("failed to update listener config secret: %w", err) } return ctrl.Result{Requeue: true}, nil @@ -467,9 +467,17 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. return ctrl.Result{}, err } - shouldReCreate := desiredPod.Annotations[annotationKeyIntegrityHash] != listenerPod.Annotations[annotationKeyIntegrityHash] - if shouldReCreate { - log.Info("Listener pod dependency changed, recreating listener pod") + 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), + ) if err := r.deleteListenerPod(ctx, &autoscalingListener, &listenerPod, log); err != nil { return ctrl.Result{}, err } @@ -485,15 +493,15 @@ func (r *AutoscalingListenerReconciler) Reconcile(ctx context.Context, req ctrl. updatedPod.Labels = desiredLabels shouldUpdate = true } - desiredAnnotations := r.mergeAnnotations(listenerPod.Annotations, desiredPod.Annotations) - if !maps.Equal(listenerPod.Annotations, desiredAnnotations) { + desiredAnnotations := r.filterAndMergeAnnotations(listenerPod.Annotations, desiredPod.Annotations) + if !r.annotationsEqual(listenerPod.Annotations, desiredAnnotations) { updatedPod.Annotations = desiredAnnotations shouldUpdate = true } if shouldUpdate { log.Info("Updating listener pod", "namespace", updatedPod.Namespace, "name", updatedPod.Name) - if err := r.Update(ctx, updatedPod); err != nil { + 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) return ctrl.Result{}, err } @@ -519,7 +527,11 @@ 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 d48d613e6c..1d8958066a 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().Update(ctx, updated) + err := k8sClient.Status().Patch(ctx, updated, client.MergeFrom(pod)) 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().Update(ctx, updated) + err := k8sClient.Status().Patch(ctx, updated, client.MergeFrom(pod)) 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().Update(ctx, updated) + err := k8sClient.Status().Patch(ctx, updated, client.MergeFrom(pod)) 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 a07cb35083..0bec0aabdf 100644 --- a/controllers/actions.github.com/autoscalingrunnerset_controller.go +++ b/controllers/actions.github.com/autoscalingrunnerset_controller.go @@ -142,13 +142,12 @@ 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 := autoscalingRunnerSet.Hash(); autoscalingRunnerSet.Annotations[annotationKeyIntegrityHash] != targetHash { - // TODO: apply the version label + if targetHash := autoscalingRunnerSetIntegrityHash(&autoscalingRunnerSet); autoscalingRunnerSet.Annotations[AnnotationKeyIntegrityHash] != targetHash { 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 @@ -291,24 +290,13 @@ func (r *AutoscalingRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl return ctrl.Result{}, nil } - 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. - if ephemeralRunnerSet.Status.RunningEphemeralRunners+ephemeralRunnerSet.Status.PendingEphemeralRunners > 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 - } - + integrityDiff := desired.Annotations[AnnotationKeyIntegrityHash] != ephemeralRunnerSetIntegrityHash(&ephemeralRunnerSet) + if integrityDiff { 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.mergeAnnotations(ephemeralRunnerSet.Annotations, desired.Annotations) + ephemeralRunnerSet.Annotations = r.filterAndMergeAnnotations(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 { @@ -322,12 +310,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 := !maps.Equal(ephemeralRunnerSet.Annotations, desired.Annotations) + ephemeralRunnerAnnotationsModified := !r.annotationsEqual(ephemeralRunnerSet.Annotations, desired.Annotations) if ephemeralRunnerLabelsModified || ephemeralRunnerAnnotationsModified || ephemeralRunnerMetadataModified { original := ephemeralRunnerSet.DeepCopy() ephemeralRunnerSet.Labels = r.filterAndMergeLabels(ephemeralRunnerSet.Labels, desired.Labels) - ephemeralRunnerSet.Annotations = r.mergeAnnotations(ephemeralRunnerSet.Annotations, desired.Annotations) + ephemeralRunnerSet.Annotations = r.filterAndMergeAnnotations(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 { @@ -370,14 +358,18 @@ func (r *AutoscalingRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl } if !cmp.Equal(listener.Spec, desired.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") + !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") return ctrl.Result{}, err } - log.Info("Deleted AutoscalingListener, will re-create on next reconcile") + log.Info("Successfully updated AutoscalingListener with new spec") 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 55ebe5c6ae..2e6053e8af 100644 --- a/controllers/actions.github.com/autoscalingrunnerset_controller_test.go +++ b/controllers/actions.github.com/autoscalingrunnerset_controller_test.go @@ -461,7 +461,14 @@ 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, @@ -472,13 +479,21 @@ 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] + originalRunnerSetHash := runnerSet.Annotations[AnnotationKeyIntegrityHash] + originalResourceVersion := runnerSet.ResourceVersion patched := autoscalingRunnerSet.DeepCopy() patched.Spec.Template.Spec.Containers[0].Image = "ghcr.io/actions/runner:updated" @@ -492,7 +507,8 @@ 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]).NotTo(Equal(originalRunnerSetHash), "EphemeralRunnerSet spec hash should change") + 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") }, autoscalingRunnerSetTestTimeout, autoscalingRunnerSetTestInterval, @@ -504,34 +520,50 @@ 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 should not be updated") + g.Expect(current.ResourceVersion).To(Equal(originalListenerResourceVersion), "Listener ResourceVersion should not change after update") }, - time.Second*5, + autoscalingRunnerSetTestTimeout, autoscalingRunnerSetTestInterval, ).Should(Succeed()) }) - It("recreates only the Listener when max runners changes", func() { + It("Updates 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") - originalRunnerSetUID := runnerSet.UID - originalRunnerSetHash := runnerSet.Annotations[annotationKeyIntegrityHash] + originalERSRunnerSetUID := runnerSet.UID + originalERSResourceVersion := runnerSet.ResourceVersion patched := autoscalingRunnerSet.DeepCopy() max := 20 @@ -544,8 +576,10 @@ 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).NotTo(Equal(originalListenerUID), "Listener should be recreated") + 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.Spec.MaxRunners).To(Equal(max)) + g.Expect(current.ResourceVersion).NotTo(Equal(originalListenerResourceVersion), "Listener ResourceVersion should change after update") }, autoscalingRunnerSetTestTimeout, autoscalingRunnerSetTestInterval, @@ -556,8 +590,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(originalRunnerSetUID), "EphemeralRunnerSet should not be recreated") - g.Expect(current.Annotations[annotationKeyIntegrityHash]).To(Equal(originalRunnerSetHash), "EphemeralRunnerSet spec should not change") + g.Expect(current.UID).To(Equal(originalERSRunnerSetUID), "EphemeralRunnerSet should not be recreated") + g.Expect(current.ResourceVersion).To(Equal(originalERSResourceVersion), "EphemeralRunnerSet spec should not change") }, time.Second*5, autoscalingRunnerSetTestInterval, @@ -568,7 +602,14 @@ 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 } @@ -586,7 +627,14 @@ 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 } @@ -601,7 +649,14 @@ 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 } @@ -614,6 +669,8 @@ 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") @@ -624,6 +681,7 @@ 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, @@ -634,7 +692,14 @@ 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")) @@ -719,22 +784,38 @@ 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") - originalRunnerSetUID := runnerSet.UID + originalEphemeralRunnerSetUID := runnerSet.UID + originalEphemeralRunnerSetIntegrityHash := runnerSet.Annotations[AnnotationKeyIntegrityHash] patched := autoscalingRunnerSet.DeepCopy() patched.Spec.GitHubConfigSecret = updatedSecret.Name @@ -757,8 +838,9 @@ 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(originalRunnerSetUID), "EphemeralRunnerSet should be updated in place") + g.Expect(current.UID).To(Equal(originalEphemeralRunnerSetUID), "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, @@ -769,8 +851,9 @@ 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).NotTo(Equal(originalListenerUID), "Listener should be recreated") - g.Expect(current.Spec.GitHubConfigSecret).To(Equal(updatedSecret.Name)) + 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") }, autoscalingRunnerSetTestTimeout, autoscalingRunnerSetTestInterval, @@ -837,111 +920,6 @@ var _ = Describe("Test AutoScalingRunnerSet controller", Ordered, func() { }) }) - 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() - activeRunnerSet.Status.CurrentReplicas = 6 - activeRunnerSet.Status.FailedEphemeralRunners = 1 - activeRunnerSet.Status.RunningEphemeralRunners = 2 - activeRunnerSet.Status.PendingEphemeralRunners = 3 - - desiredStatus := v1alpha1.AutoscalingRunnerSetStatus{ - CurrentRunners: activeRunnerSet.Status.CurrentReplicas, - Phase: v1alpha1.AutoscalingRunnerSetPhaseRunning, - PendingEphemeralRunners: activeRunnerSet.Status.PendingEphemeralRunners, - RunningEphemeralRunners: activeRunnerSet.Status.RunningEphemeralRunners, - FailedEphemeralRunners: activeRunnerSet.Status.FailedEphemeralRunners, - } - - err = k8sClient.Status().Patch(ctx, activeRunnerSet, 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") - - // 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", - }, - }, - } - err = k8sClient.Patch(ctx, patched, client.MergeFrom(autoscalingRunnerSet)) - Expect(err).NotTo(HaveOccurred(), "failed to patch AutoScalingRunnerSet") - autoscalingRunnerSet = patched.DeepCopy() - - // 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") - - // 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") - }) - }) - It("Should update Status on EphemeralRunnerSet status Update", func() { ars := new(v1alpha1.AutoscalingRunnerSet) Eventually( @@ -1211,10 +1189,11 @@ var _ = Describe("Test AutoscalingController creation failures", Ordered, func() }, }, Spec: v1alpha1.AutoscalingRunnerSetSpec{ - GitHubConfigUrl: "https://github.com/owner/repo", - MaxRunners: &max, - MinRunners: &min, - RunnerGroup: "testgroup", + GitHubConfigUrl: "https://github.com/owner/repo", + GitHubConfigSecret: "secret1", + MaxRunners: &max, + MinRunners: &min, + RunnerGroup: "testgroup", Template: corev1.PodTemplateSpec{ Spec: corev1.PodSpec{ Containers: []corev1.Container{ @@ -1248,8 +1227,9 @@ var _ = Describe("Test AutoscalingController creation failures", Ordered, func() autoscalingRunnerSetTestInterval, ).Should(BeEquivalentTo(autoscalingRunnerSetFinalizerName), "AutoScalingRunnerSet should have a finalizer") - ars.Annotations = make(map[string]string) - err = k8sClient.Update(ctx, ars) + updated := ars.DeepCopy() + updated.Annotations = make(map[string]string) + err = k8sClient.Patch(ctx, updated, client.MergeFrom(ars)) Expect(err).NotTo(HaveOccurred(), "Update autoscaling runner set without annotation should be successful") Eventually( @@ -1353,7 +1333,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{ @@ -1431,7 +1411,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 80c27134a7..506355acf8 100644 --- a/controllers/actions.github.com/ephemeralrunner_controller_test.go +++ b/controllers/actions.github.com/ephemeralrunner_controller_test.go @@ -223,8 +223,9 @@ var _ = Describe("EphemeralRunner", func() { ).Should(Succeed(), "failed to get ephemeral runner") // update job id to simulate job assigned - er.Status.JobID = "1" - err := k8sClient.Status().Update(ctx, er) + updatedER := er.DeepCopy() + updatedER.Status.JobID = "1" + err := k8sClient.Status().Patch(ctx, updatedER, client.MergeFrom(er)) Expect(err).To(BeNil(), "failed to update ephemeral runner status") er = new(v1alpha1.EphemeralRunner) @@ -249,7 +250,8 @@ var _ = Describe("EphemeralRunner", func() { }).Should(BeEquivalentTo(true)) // delete pod to simulate failure - pod.Status.ContainerStatuses = append(pod.Status.ContainerStatuses, corev1.ContainerStatus{ + updatedPod := pod.DeepCopy() + updatedPod.Status.ContainerStatuses = append(updatedPod.Status.ContainerStatuses, corev1.ContainerStatus{ Name: v1alpha1.EphemeralRunnerContainerName, State: corev1.ContainerState{ Terminated: &corev1.ContainerStateTerminated{ @@ -257,7 +259,7 @@ var _ = Describe("EphemeralRunner", func() { }, }, }) - err = k8sClient.Status().Update(ctx, pod) + err = k8sClient.Status().Patch(ctx, updatedPod, client.MergeFrom(pod)) Expect(err).To(BeNil(), "Failed to update pod status") er = new(v1alpha1.EphemeralRunner) @@ -277,8 +279,9 @@ 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") - er.Status.JobID = "1" - err := k8sClient.Status().Update(ctx, er) + updatedER := er.DeepCopy() + updatedER.Status.JobID = "1" + err := k8sClient.Status().Patch(ctx, updatedER, client.MergeFrom(er)) Expect(err).To(BeNil(), "failed to update ephemeral runner status") Eventually(func() (string, error) { @@ -297,9 +300,10 @@ var _ = Describe("EphemeralRunner", func() { return true, nil }, ephemeralRunnerTimeout, ephemeralRunnerInterval).Should(BeEquivalentTo(true)) - pod.Status.Phase = corev1.PodFailed - pod.Status.ContainerStatuses = nil - err = k8sClient.Status().Update(ctx, pod) + updatedPod := pod.DeepCopy() + updatedPod.Status.Phase = corev1.PodFailed + updatedPod.Status.ContainerStatuses = nil + err = k8sClient.Status().Patch(ctx, updatedPod, client.MergeFrom(pod)) Expect(err).To(BeNil(), "Failed to update pod status") Eventually(func() bool { @@ -320,9 +324,10 @@ var _ = Describe("EphemeralRunner", func() { oldPodUID := pod.UID - pod.Status.Phase = corev1.PodFailed - pod.Status.ContainerStatuses = nil - err := k8sClient.Status().Update(ctx, pod) + updatedPod := pod.DeepCopy() + updatedPod.Status.Phase = corev1.PodFailed + updatedPod.Status.ContainerStatuses = nil + err := k8sClient.Status().Patch(ctx, updatedPod, client.MergeFrom(pod)) Expect(err).To(BeNil(), "Failed to update pod status") Eventually( @@ -369,8 +374,9 @@ var _ = Describe("EphemeralRunner", func() { // Simulate init container failure without PodFailed phase. // This can happen when the kubelet has not yet transitioned the pod phase. - pod.Status.Phase = corev1.PodPending - pod.Status.InitContainerStatuses = []corev1.ContainerStatus{ + updatedPod := pod.DeepCopy() + updatedPod.Status.Phase = corev1.PodPending + updatedPod.Status.InitContainerStatuses = []corev1.ContainerStatus{ { Name: "setup", State: corev1.ContainerState{ @@ -382,7 +388,7 @@ var _ = Describe("EphemeralRunner", func() { }, }, } - err := k8sClient.Status().Update(ctx, pod) + err := k8sClient.Status().Patch(ctx, updatedPod, client.MergeFrom(pod)) Expect(err).To(BeNil(), "Failed to update pod status") Eventually( @@ -422,8 +428,9 @@ 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") - er.Status.JobID = "1" - err := k8sClient.Status().Update(ctx, er) + updatedER := er.DeepCopy() + updatedER.Status.JobID = "1" + err := k8sClient.Status().Patch(ctx, updatedER, client.MergeFrom(er)) Expect(err).To(BeNil(), "failed to update ephemeral runner status") Eventually(func() (string, error) { @@ -443,8 +450,9 @@ var _ = Describe("EphemeralRunner", func() { }, ephemeralRunnerTimeout, ephemeralRunnerInterval).Should(BeEquivalentTo(true)) // Simulate init container failure with job assigned - pod.Status.Phase = corev1.PodPending - pod.Status.InitContainerStatuses = []corev1.ContainerStatus{ + updatedPod := pod.DeepCopy() + updatedPod.Status.Phase = corev1.PodPending + updatedPod.Status.InitContainerStatuses = []corev1.ContainerStatus{ { Name: "setup", State: corev1.ContainerState{ @@ -455,7 +463,7 @@ var _ = Describe("EphemeralRunner", func() { }, }, } - err = k8sClient.Status().Update(ctx, pod) + err = k8sClient.Status().Patch(ctx, updatedPod, client.MergeFrom(pod)) Expect(err).To(BeNil(), "Failed to update pod status") Eventually(func() bool { @@ -471,8 +479,9 @@ 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") - er.Status.JobID = "1" - err := k8sClient.Status().Update(ctx, er) + updatedER := er.DeepCopy() + updatedER.Status.JobID = "1" + err := k8sClient.Status().Patch(ctx, updatedER, client.MergeFrom(er)) Expect(err).To(BeNil(), "failed to update ephemeral runner status") pod := new(corev1.Pod) @@ -487,8 +496,9 @@ var _ = Describe("EphemeralRunner", func() { ephemeralRunnerInterval, ).Should(Succeed(), "failed to get pod") - pod.Status.Phase = corev1.PodFailed - pod.Status.ContainerStatuses = append(pod.Status.ContainerStatuses, corev1.ContainerStatus{ + updatedPod := pod.DeepCopy() + updatedPod.Status.Phase = corev1.PodFailed + updatedPod.Status.ContainerStatuses = append(updatedPod.Status.ContainerStatuses, corev1.ContainerStatus{ Name: v1alpha1.EphemeralRunnerContainerName, State: corev1.ContainerState{ Terminated: &corev1.ContainerStateTerminated{ @@ -496,7 +506,7 @@ var _ = Describe("EphemeralRunner", func() { }, }, }) - err = k8sClient.Status().Update(ctx, pod) + err = k8sClient.Status().Patch(ctx, updatedPod, client.MergeFrom(pod)) Expect(err).To(BeNil(), "Failed to update pod status") Eventually( @@ -523,8 +533,9 @@ var _ = Describe("EphemeralRunner", func() { ephemeralRunnerInterval, ).Should(Succeed(), "failed to get pod") - pod.Status.Phase = corev1.PodFailed - pod.Status.ContainerStatuses = append(pod.Status.ContainerStatuses, corev1.ContainerStatus{ + updatedPod := pod.DeepCopy() + updatedPod.Status.Phase = corev1.PodFailed + updatedPod.Status.ContainerStatuses = append(updatedPod.Status.ContainerStatuses, corev1.ContainerStatus{ Name: v1alpha1.EphemeralRunnerContainerName, State: corev1.ContainerState{ Terminated: &corev1.ContainerStateTerminated{ @@ -532,7 +543,7 @@ var _ = Describe("EphemeralRunner", func() { }, }, }) - err := k8sClient.Status().Update(ctx, pod) + err := k8sClient.Status().Patch(ctx, updatedPod, client.MergeFrom(pod)) Expect(err).To(BeNil(), "Failed to update pod status") Eventually( @@ -568,9 +579,10 @@ var _ = Describe("EphemeralRunner", func() { ephemeralRunnerInterval, ).Should(Succeed(), "failed to get pod") - pod.Status.Phase = corev1.PodFailed + updatedPod := pod.DeepCopy() + updatedPod.Status.Phase = corev1.PodFailed oldPodUID := pod.UID - pod.Status.ContainerStatuses = append(pod.Status.ContainerStatuses, corev1.ContainerStatus{ + updatedPod.Status.ContainerStatuses = append(updatedPod.Status.ContainerStatuses, corev1.ContainerStatus{ Name: v1alpha1.EphemeralRunnerContainerName, State: corev1.ContainerState{ Terminated: &corev1.ContainerStateTerminated{ @@ -579,7 +591,7 @@ var _ = Describe("EphemeralRunner", func() { }, }) - err := k8sClient.Status().Update(ctx, pod) + err := k8sClient.Status().Patch(ctx, updatedPod, client.MergeFrom(pod)) Expect(err).To(BeNil(), "Failed to update pod status") Eventually( @@ -939,8 +951,9 @@ var _ = Describe("EphemeralRunner", func() { ephemeralRunnerInterval, ).Should(BeEquivalentTo(true)) - pod.Status.Phase = corev1.PodRunning - err := k8sClient.Status().Update(ctx, pod) + updatedPod := pod.DeepCopy() + updatedPod.Status.Phase = corev1.PodRunning + err := k8sClient.Status().Patch(ctx, updatedPod, client.MergeFrom(pod)) Expect(err).To(BeNil(), "failed to patch pod status") Consistently( @@ -975,7 +988,8 @@ var _ = Describe("EphemeralRunner", func() { ephemeralRunnerInterval, ).Should(Succeed(), "failed to get ephemeral runner pod") - pod.Status.ContainerStatuses = append(pod.Status.ContainerStatuses, corev1.ContainerStatus{ + updatedPod := pod.DeepCopy() + updatedPod.Status.ContainerStatuses = append(updatedPod.Status.ContainerStatuses, corev1.ContainerStatus{ Name: v1alpha1.EphemeralRunnerContainerName, State: corev1.ContainerState{ Terminated: &corev1.ContainerStateTerminated{ @@ -983,10 +997,10 @@ var _ = Describe("EphemeralRunner", func() { }, }, }) - err := k8sClient.Status().Update(ctx, pod) + err := k8sClient.Status().Patch(ctx, updatedPod, client.MergeFrom(pod)) Expect(err).To(BeNil(), "Failed to update pod status") - return pod + return updatedPod } for i := range 5 { @@ -1065,13 +1079,14 @@ var _ = Describe("EphemeralRunner", func() { ephemeralRunnerInterval, ).Should(BeEquivalentTo(true)) - pod.Status.Phase = corev1.PodFailed - pod.Status.Reason = "Evicted" - pod.Status.ContainerStatuses = append(pod.Status.ContainerStatuses, corev1.ContainerStatus{ + updatedPod := pod.DeepCopy() + updatedPod.Status.Phase = corev1.PodFailed + updatedPod.Status.Reason = "Evicted" + updatedPod.Status.ContainerStatuses = append(updatedPod.Status.ContainerStatuses, corev1.ContainerStatus{ Name: v1alpha1.EphemeralRunnerContainerName, State: corev1.ContainerState{}, }) - err := k8sClient.Status().Update(ctx, pod) + err := k8sClient.Status().Patch(ctx, updatedPod, client.MergeFrom(pod)) Expect(err).To(BeNil(), "failed to patch pod status") updated := new(v1alpha1.EphemeralRunner) @@ -1111,13 +1126,14 @@ var _ = Describe("EphemeralRunner", func() { ephemeralRunnerInterval, ).Should(BeEquivalentTo(true)) - pod.Status.Phase = corev1.PodFailed - pod.Status.Reason = "OutOfpods" - pod.Status.ContainerStatuses = append(pod.Status.ContainerStatuses, corev1.ContainerStatus{ + updatedPod := pod.DeepCopy() + updatedPod.Status.Phase = corev1.PodFailed + updatedPod.Status.Reason = "OutOfpods" + updatedPod.Status.ContainerStatuses = append(updatedPod.Status.ContainerStatuses, corev1.ContainerStatus{ Name: v1alpha1.EphemeralRunnerContainerName, State: corev1.ContainerState{}, }) - err := k8sClient.Status().Update(ctx, pod) + err := k8sClient.Status().Patch(ctx, updatedPod, client.MergeFrom(pod)) Expect(err).To(BeNil(), "failed to patch pod status") updated := new(v1alpha1.EphemeralRunner) @@ -1157,7 +1173,8 @@ var _ = Describe("EphemeralRunner", func() { ).Should(BeEquivalentTo(true)) // first set phase to running - pod.Status.ContainerStatuses = append(pod.Status.ContainerStatuses, corev1.ContainerStatus{ + updatedPod := pod.DeepCopy() + updatedPod.Status.ContainerStatuses = append(updatedPod.Status.ContainerStatuses, corev1.ContainerStatus{ Name: v1alpha1.EphemeralRunnerContainerName, State: corev1.ContainerState{ Running: &corev1.ContainerStateRunning{ @@ -1165,8 +1182,8 @@ var _ = Describe("EphemeralRunner", func() { }, }, }) - pod.Status.Phase = corev1.PodRunning - err := k8sClient.Status().Update(ctx, pod) + updatedPod.Status.Phase = corev1.PodRunning + err := k8sClient.Status().Patch(ctx, updatedPod, client.MergeFrom(pod)) Expect(err).To(BeNil()) Eventually( @@ -1182,8 +1199,9 @@ var _ = Describe("EphemeralRunner", func() { ).Should(BeEquivalentTo(v1alpha1.EphemeralRunnerPhaseRunning)) // set phase to succeeded - pod.Status.Phase = corev1.PodSucceeded - err = k8sClient.Status().Update(ctx, pod) + nextPod := updatedPod.DeepCopy() + nextPod.Status.Phase = corev1.PodSucceeded + err = k8sClient.Status().Patch(ctx, nextPod, client.MergeFrom(updatedPod)) Expect(err).To(BeNil()) Consistently( @@ -1258,7 +1276,8 @@ var _ = Describe("EphemeralRunner", func() { return true, nil }, ephemeralRunnerTimeout, ephemeralRunnerInterval).Should(BeEquivalentTo(true)) - pod.Status.ContainerStatuses = append(pod.Status.ContainerStatuses, corev1.ContainerStatus{ + updatedPod := pod.DeepCopy() + updatedPod.Status.ContainerStatuses = append(updatedPod.Status.ContainerStatuses, corev1.ContainerStatus{ Name: v1alpha1.EphemeralRunnerContainerName, State: corev1.ContainerState{ Terminated: &corev1.ContainerStateTerminated{ @@ -1266,7 +1285,7 @@ var _ = Describe("EphemeralRunner", func() { }, }, }) - err = k8sClient.Status().Update(ctx, pod) + err = k8sClient.Status().Patch(ctx, updatedPod, client.MergeFrom(pod)) Expect(err).To(BeNil(), "failed to update pod status") updated := new(v1alpha1.EphemeralRunner) @@ -1367,7 +1386,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", }, } @@ -1388,10 +1407,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.go b/controllers/actions.github.com/ephemeralrunnerset_controller.go index 381c9d29de..a062d56fd5 100644 --- a/controllers/actions.github.com/ephemeralrunnerset_controller.go +++ b/controllers/actions.github.com/ephemeralrunnerset_controller.go @@ -138,7 +138,7 @@ func (r *EphemeralRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl.R // If hash spec has changed, delete idle ephemeral runners // in order to apply the change to the runners that did not yet receive a job. ephemeralRunnerIntegrityHash := ephemeralRunnerSetIntegrityHash(&ephemeralRunnerSet) - if ephemeralRunnerSet.Annotations[annotationKeyIntegrityHash] != ephemeralRunnerIntegrityHash { + if ephemeralRunnerSet.Annotations[AnnotationKeyIntegrityHash] != ephemeralRunnerIntegrityHash { log.Info("EphemeralRunnerSpec has changed, deleting idle ephemeral runners to apply the new spec") if _, err := r.cleanUpEphemeralRunners(ctx, &ephemeralRunnerSet, log); err != nil { log.Error(err, "Failed to clean up EphemeralRunners") @@ -155,7 +155,7 @@ func (r *EphemeralRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl.R if ephemeralRunnerSet.Annotations == nil { ephemeralRunnerSet.Annotations = make(map[string]string) } - ephemeralRunnerSet.Annotations[annotationKeyIntegrityHash] = ephemeralRunnerIntegrityHash + ephemeralRunnerSet.Annotations[AnnotationKeyIntegrityHash] = ephemeralRunnerIntegrityHash if err := r.Patch(ctx, &ephemeralRunnerSet, client.MergeFrom(original)); err != nil { log.Error(err, "Failed to update ephemeral runner set with new spec hash") return ctrl.Result{}, err @@ -271,7 +271,6 @@ func (r *EphemeralRunnerSetReconciler) Reconcile(ctx context.Context, req ctrl.R } func (r *EphemeralRunnerSetReconciler) updateStatus(ctx context.Context, ephemeralRunnerSet *v1alpha1.EphemeralRunnerSet, state *ephemeralRunnersByState, log logr.Logger) error { - original := ephemeralRunnerSet.DeepCopy() total := state.scaleTotal() var phase v1alpha1.EphemeralRunnerSetPhase switch { @@ -292,12 +291,13 @@ func (r *EphemeralRunnerSetReconciler) updateStatus(ctx context.Context, ephemer // Update the status if needed. if ephemeralRunnerSet.Status != desiredStatus { - ephemeralRunnerSet.Status = desiredStatus - if err := r.Status().Patch(ctx, ephemeralRunnerSet, client.MergeFrom(original)); err != nil { + updated := ephemeralRunnerSet.DeepCopy() + updated.Status = desiredStatus + if err := r.Status().Patch(ctx, updated, client.MergeFrom(ephemeralRunnerSet)); err != nil { log.Error(err, "Failed to update EphemeralRunnerSet status") return err } - log.Info("Updated EphemeralRunnerSet status", "status", ephemeralRunnerSet.Status) + log.Info("Updated EphemeralRunnerSet status", "status", updated.Status) } return nil @@ -507,14 +507,14 @@ func (r *EphemeralRunnerSetReconciler) reconcileEphemeralRunnerSetProxySecret(ct updatedProxySecret.Labels = desiredLabels shouldUpdate = true } - desiredAnnotations := r.mergeAnnotations(proxySecret.Annotations, desiredRunnerSetProxy.Annotations) + desiredAnnotations := r.filterAndMergeAnnotations(proxySecret.Annotations, desiredRunnerSetProxy.Annotations) if !maps.Equal(proxySecret.Annotations, desiredAnnotations) { updatedProxySecret.Annotations = desiredAnnotations shouldUpdate = true } if shouldUpdate { log.Info("Updating ephemeralRunnerSet proxy secret") - if err := r.Update(ctx, updatedProxySecret); err != nil { + if err := r.Patch(ctx, updatedProxySecret, client.MergeFrom(&proxySecret)); err != nil { return nil, false, fmt.Errorf("failed to update ephemeralRunnerSet proxy secret: %w", err) } return updatedProxySecret, true, nil diff --git a/controllers/actions.github.com/ephemeralrunnerset_controller_test.go b/controllers/actions.github.com/ephemeralrunnerset_controller_test.go index c345965256..a395b786cb 100644 --- a/controllers/actions.github.com/ephemeralrunnerset_controller_test.go +++ b/controllers/actions.github.com/ephemeralrunnerset_controller_test.go @@ -151,7 +151,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { // Scaling up the EphemeralRunnerSet updated := created.DeepCopy() updated.Spec.Replicas = 5 - err := k8sClient.Update(ctx, updated) + err := k8sClient.Patch(ctx, updated, client.MergeFrom(created)) Expect(err).NotTo(HaveOccurred(), "failed to update EphemeralRunnerSet") // Check if the number of ephemeral runners are created @@ -213,7 +213,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { // Scale up the EphemeralRunnerSet updated := created.DeepCopy() updated.Spec.Replicas = 5 - err = k8sClient.Update(ctx, updated) + err = k8sClient.Patch(ctx, updated, client.MergeFrom(created)) Expect(err).NotTo(HaveOccurred(), "failed to update EphemeralRunnerSet") // Wait for the EphemeralRunnerSet to be scaled up @@ -1101,7 +1101,7 @@ var _ = Describe("Test EphemeralRunnerSet controller", func() { // Scale up the EphemeralRunnerSet updated := created.DeepCopy() updated.Spec.Replicas = 3 - err := k8sClient.Update(ctx, updated) + err := k8sClient.Patch(ctx, updated, client.MergeFrom(created)) Expect(err).NotTo(HaveOccurred(), "failed to update EphemeralRunnerSet replica count") runnerList := new(v1alpha1.EphemeralRunnerList) @@ -1327,11 +1327,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"}, @@ -1510,7 +1510,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 fd456b56f1..68ce5cbc69 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,6 +115,10 @@ 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 { @@ -122,13 +126,14 @@ 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, @@ -165,12 +170,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.mergeAnnotations(autoscalingRunnerSet.Spec.AutoscalingListenerMetadata.Annotations, annotations) + annotations = b.filterAndMergeAnnotations(autoscalingRunnerSet.Spec.AutoscalingListenerMetadata.Annotations, annotations) } autoscalingListener := &v1alpha1.AutoscalingListener{ @@ -278,7 +283,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) @@ -426,7 +431,7 @@ func (b *ResourceBuilder) newScaleSetListenerPod( Spec: podSpec, } - newRunnerScaleSetListenerPod.Annotations[annotationKeyIntegrityHash] = scaleSetListenerPodIntegrity( + newRunnerScaleSetListenerPod.Annotations[AnnotationKeyIntegrityHash] = scaleSetListenerPodIntegrity( newRunnerScaleSetListenerPod, autoscalingListener, podConfig, @@ -468,11 +473,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, } @@ -611,10 +616,10 @@ func (b *ResourceBuilder) newScaleSetListenerServiceAccount(autoscalingListener if autoscalingListener.Spec.ServiceAccountMetadata != nil { base.Labels = b.filterAndMergeLabels(autoscalingListener.Spec.ServiceAccountMetadata.Labels, base.Labels) - base.Annotations = b.mergeAnnotations(autoscalingListener.Spec.ServiceAccountMetadata.Annotations, base.Annotations) + base.Annotations = b.filterAndMergeAnnotations(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) @@ -650,7 +655,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.mergeAnnotations(autoscalingListener.Spec.RoleMetadata.Annotations, nil) + annotations = b.filterAndMergeAnnotations(autoscalingListener.Spec.RoleMetadata.Annotations, nil) } newRole := &rbacv1.Role{ @@ -663,7 +668,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 } @@ -718,7 +723,7 @@ func (b *ResourceBuilder) newScaleSetListenerRoleBinding(autoscalingListener *v1 Subjects: subjects, } - newRoleBinding.Annotations[annotationKeyIntegrityHash] = scaleSetListenerRoleBindingIntegrityHash(newRoleBinding) + newRoleBinding.Annotations[AnnotationKeyIntegrityHash] = scaleSetListenerRoleBindingIntegrityHash(newRoleBinding) return newRoleBinding } @@ -777,7 +782,7 @@ func (b *ResourceBuilder) newEphemeralRunnerSet(autoscalingRunnerSet *v1alpha1.A if autoscalingRunnerSet.Spec.EphemeralRunnerSetMetadata != nil { labels = b.filterAndMergeLabels(autoscalingRunnerSet.Spec.EphemeralRunnerSetMetadata.Labels, labels) - annotations = b.mergeAnnotations(autoscalingRunnerSet.Spec.EphemeralRunnerSetMetadata.Annotations, annotations) + annotations = b.filterAndMergeAnnotations(autoscalingRunnerSet.Spec.EphemeralRunnerSetMetadata.Annotations, annotations) } newEphemeralRunnerSet := &v1alpha1.EphemeralRunnerSet{ @@ -791,7 +796,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) @@ -825,7 +830,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) @@ -857,7 +862,7 @@ func (b *ResourceBuilder) newEphemeralRunner(ephemeralRunnerSet *v1alpha1.Epheme if ephemeralRunnerSet.Spec.EphemeralRunnerMetadata != nil { labels = b.filterAndMergeLabels(ephemeralRunnerSet.Spec.EphemeralRunnerMetadata.Labels, labels) - annotations = b.mergeAnnotations(ephemeralRunnerSet.Spec.EphemeralRunnerMetadata.Annotations, annotations) + annotations = b.filterAndMergeAnnotations(ephemeralRunnerSet.Spec.EphemeralRunnerMetadata.Annotations, annotations) } ephemeralRunner := &v1alpha1.EphemeralRunner{ @@ -992,7 +997,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) @@ -1093,40 +1098,78 @@ 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 +} - 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 +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) } -overwrite: for k, v := range overwrite { - for _, prefix := range b.ExcludeLabelPropagationPrefixes { - if strings.HasPrefix(k, prefix) { - continue overwrite - } + if k == AnnotationKeyIntegrityHash { + continue } - mergedLabels[k] = v + result[k] = v } - return mergedLabels + return result } -func (b *ResourceBuilder) mergeAnnotations(base, overwrite map[string]string) map[string]string { - if base == nil && overwrite == nil { - return nil +// 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++ } - base = maps.Clone(base) - maps.Copy(base, overwrite) - return base + 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 } diff --git a/controllers/actions.github.com/resourcebuilder_test.go b/controllers/actions.github.com/resourcebuilder_test.go index d08851173a..206e610748 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 5f3e95612d..b9f9d9ccba 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 a77b24ba17..da87eb7b21 100644 --- a/controllers/actions.github.com/utils.go +++ b/controllers/actions.github.com/utils.go @@ -1,6 +1,8 @@ package actionsgithubcom import ( + "encoding/json" + "k8s.io/apimachinery/pkg/util/rand" ) @@ -25,3 +27,11 @@ 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) +}