new forgejo scaler #6495

cobak78 · 2025-01-22T14:40:16Z

This PR adds a new forgejo autoscaler with the same characteristics of the github autoscaler.
Here is the issue for this feature #6488

Another important discussions on this topic:

The issue on forgejo with the new endpoint needed to make this autoscaler work. https://codeberg.org/forgejo/discussions/issues/241
The accepted PR https://codeberg.org/forgejo/forgejo/pulls/6300
The modifications on the forgejo runner to work as a scaled job. https://code.forgejo.org/forgejo/runner/pulls/423

Checklist

When introducing a new scaler, I agree with the scaling governance policy
I have verified that my change is according to the deprecations & breaking changes policy
Tests have been added
Changelog has been updated and is aligned with our changelog requirements
A PR is opened to update our Helm chart (repo) (if applicable, ie. when deployment manifests are modified)
A PR is opened to update the documentation on (repo) (if applicable)
Commits are signed with Developer Certificate of Origin (DCO - learn more)

pkg/scalers/forgejo_runner_scaler_test.go

pkg/scalers/forgejo_runner_scaler.go

pkg/scalers/forgejo_runner_scaler_test.go

Signed-off-by: jaime Merino <[email protected]>

Signed-off-by: jaime merino <[email protected]ª>

JorTurFer

Thanks a lot for your contribution! I've left some comment inline. Additionally to them:

Please, add an e2e test covering the scaler. You get more info about how to do it here
Could you open a PR to docs adding the scaler in next version?
All commits must be signed and that's why DCO check is failing, please rebase your branch signing all the commits (how to do that it's explained in the check page)

JorTurFer · 2025-01-24T18:38:38Z

pkg/scalers/forgejo/action.go

@@ -0,0 +1,43 @@
+/*


As this is just a type definition file, I think that it's better if it's merged with the scaler code. We create packages when there is complex logic or specialized helpers, but just to define some structs, we prefer to keep all together

JorTurFer · 2025-01-24T18:39:42Z

pkg/scalers/forgejo_runner_scaler.go

+	orgJobMetricPath   = "/api/v1/orgs/%s/actions/runners/jobs"     // /api/v1/orgs/{org}/actions/runners/jobs
+	repoJobMetricPath  = "/api/v1/repos/%s/%s/actions/runners/jobs" // {address}/api/v1/repos/{owner}/{repo}/actions/runners/jobs


Suggested change

orgJobMetricPath = "/api/v1/orgs/%s/actions/runners/jobs" // /api/v1/orgs/{org}/actions/runners/jobs

repoJobMetricPath = "/api/v1/repos/%s/%s/actions/runners/jobs" // {address}/api/v1/repos/{owner}/{repo}/actions/runners/jobs

orgJobMetricPath = "/api/v1/orgs/%s/actions/runners/jobs"

repoJobMetricPath = "/api/v1/repos/%s/%s/actions/runners/jobs"

JorTurFer · 2025-01-24T18:43:31Z

pkg/scalers/forgejo_runner_scaler.go

+func parseForgejoRunnerMetadata(config *scalersconfig.ScalerConfig) (*ForgejoRunnerConfig, error) {
+	meta := &ForgejoRunnerConfig{}
+
+	if val, ok := config.TriggerMetadata["token"]; ok && val != "" {
+		meta.RunnerMeta.Token = val
+	} else {
+		return nil, fmt.Errorf("no token given")
+	}
+
+	if val, ok := config.TriggerMetadata["address"]; ok && val != "" {
+		meta.RunnerMeta.Address = val
+	} else {
+		return nil, fmt.Errorf("no address given")
+	}
+
+	if val, ok := config.TriggerMetadata["labels"]; ok && val != "" {
+		meta.RunnerMeta.Labels = val
+	} else {
+		return nil, fmt.Errorf("no labels given")
+	}
+
+	global := false
+	if val, ok := config.TriggerMetadata["global"]; ok && val == stringTrue {
+		global = true
+	}
+
+	meta.RunnerMeta.Global = global
+
+	return meta, nil
+}


Please, use the new declarative way to define the scaler metadata. We are moving all the scalers to there to build tools around the approach.

You can see an example with other scalers like https://github.com/kedacore/keda/pull/5818/files

JorTurFer · 2025-01-24T18:45:42Z

pkg/scalers/forgejo_runner_scaler.go

+	externalMetric := &v2.ExternalMetricSource{
+		Metric: v2.MetricIdentifier{
+			Name: GenerateMetricNameWithIndex(
+				1,


This value is provided via config.TriggerIndex during the scaler building process, please propagate it until here as it's required to ensure that each scaler has a unique name within the ScaledObject|Job

JorTurFer · 2025-01-24T18:47:46Z

pkg/scalers/forgejo_runner_scaler.go

+		Metric: v2.MetricIdentifier{
+			Name: GenerateMetricNameWithIndex(
+				1,
+				kedautil.NormalizeString(fmt.Sprintf("forgejo-runner-%s", s.metadata.RunnerMeta.Address)),


This will be used as metric name, so we must ensure that it's a sanitized string (it'll be sent via url). What about just using "forgejo-runner" or concated labels?

JorTurFer · 2025-01-24T18:59:26Z

pkg/scalers/forgejo_runner_scaler.go

+	adminJobMetricPath = "/api/v1/admin/runners/jobs"
+	orgJobMetricPath   = "/api/v1/orgs/%s/actions/runners/jobs"     // /api/v1/orgs/{org}/actions/runners/jobs
+	repoJobMetricPath  = "/api/v1/repos/%s/%s/actions/runners/jobs" // {address}/api/v1/repos/{owner}/{repo}/actions/runners/jobs
+	UserJobMetricPath  = "/api/v1/user/actions/runners/jobs"


Suggested change

UserJobMetricPath = "/api/v1/user/actions/runners/jobs"

userJobMetricPath = "/api/v1/user/actions/runners/jobs"

JorTurFer · 2025-01-24T19:01:16Z

pkg/scalers/forgejo_runner_scaler.go

+
+	metric := GenerateMetricInMili(metricName, float64(len(jobList.Jobs)))
+
+	metric.Value.Add(resource.Quantity{})


Why do you need this? I'd say that it's required because GenerateMetricInMili returns a formatted metric

JorTurFer · 2025-01-24T19:04:38Z

pkg/scalers/forgejo_runner_scaler.go

+	err = json.Unmarshal(b, &jobList)
+	return jobList, err


I have no idea about fogejo API, does it only return pending jobs, or it also returns active/completed jobs somehow? My question is related with the point about if we need to filter the jobs somehow, or just counting them is enough.

JorTurFer · 2025-01-24T19:06:41Z

pkg/scalers/forgejo_runner_scaler_test.go

@@ -0,0 +1,266 @@
+package scalers


Let's use Pascal case for test naming to be consistent with other test cases

JorTurFer · 2025-01-24T19:08:44Z

pkg/scalers/forgejo_runner_scaler_test.go

+	{"no address given", map[string]string{
+		"token": "some-token", "name": "some-name", "labels": "some-label", "metric_path": "path",
+	}, true, "no address given", false},
+	{"no token given", map[string]string{
+		"address": "https://api.github.com", "name": "some-name", "labels": "some-label", "metric_path": "path",
+	}, true, "no token given", false},
+	{"no label given", map[string]string{
+		"address": "https://api.github.com", "token": "some-token", "name": "some-name", "metric_path": "path",
+	}, true, "no labels given", false},
+	{"no name given use default", map[string]string{
+		"address": "https://api.github.com", "token": "some-token", "labels": "some-label", "metric_path": "path",
+	}, false, "", false},
+	{"no metric path given use default", map[string]string{
+		"address": "https://api.github.com", "token": "some-token", "name": "some-name", "labels": "some-label",
+	}, false, "", false},
+	// properly formed
+	{"properly formed", map[string]string{
+		"address":     "https://api.github.com",
+		"token":       "some-token",
+		"name":        "some-name",
+		"labels":      "some-labels",
+		"metric_path": "path",
+	}, false, "", false},
+	{"properly formed with global", map[string]string{
+		"address":     "https://api.github.com",
+		"token":       "some-token",
+		"name":        "some-name",
+		"labels":      "some-labels",
+		"global":      "true",
+		"metric_path": "path",
+	}, false, "", true},
+}


I guess that the url isn't relevant for this test case but let's use other unrelated with GitHub to avoid misunderstandings or confusions with GitHub scaler

Suggested change

{"no address given", map[string]string{

"token": "some-token", "name": "some-name", "labels": "some-label", "metric_path": "path",

}, true, "no address given", false},

{"no token given", map[string]string{

"address": "https://api.github.com", "name": "some-name", "labels": "some-label", "metric_path": "path",

}, true, "no token given", false},

{"no label given", map[string]string{

"address": "https://api.github.com", "token": "some-token", "name": "some-name", "metric_path": "path",

}, true, "no labels given", false},

{"no name given use default", map[string]string{

"address": "https://api.github.com", "token": "some-token", "labels": "some-label", "metric_path": "path",

}, false, "", false},

{"no metric path given use default", map[string]string{

"address": "https://api.github.com", "token": "some-token", "name": "some-name", "labels": "some-label",

}, false, "", false},

// properly formed

{"properly formed", map[string]string{

"address": "https://api.github.com",

"token": "some-token",

"name": "some-name",

"labels": "some-labels",

"metric_path": "path",

}, false, "", false},

{"properly formed with global", map[string]string{

"address": "https://api.github.com",

"token": "some-token",

"name": "some-name",

"labels": "some-labels",

"global": "true",

"metric_path": "path",

}, false, "", true},

}

{"no address given", map[string]string{

"token": "some-token", "name": "some-name", "labels": "some-label", "metric_path": "path",

}, true, "no address given", false},

{"no token given", map[string]string{

"address": "https://api.myforgejoinstance.com", "name": "some-name", "labels": "some-label", "metric_path": "path",

}, true, "no token given", false},

{"no label given", map[string]string{

"address": "https://api.myforgejoinstance.com", "token": "some-token", "name": "some-name", "metric_path": "path",

}, true, "no labels given", false},

{"no name given use default", map[string]string{

"address": "https://api.myforgejoinstance.com", "token": "some-token", "labels": "some-label", "metric_path": "path",

}, false, "", false},

{"no metric path given use default", map[string]string{

"address": "https://api.myforgejoinstance.com", "token": "some-token", "name": "some-name", "labels": "some-label",

}, false, "", false},

// properly formed

{"properly formed", map[string]string{

"address": "https://api.myforgejoinstance.com",

"token": "some-token",

"name": "some-name",

"labels": "some-labels",

"metric_path": "path",

}, false, "", false},

{"properly formed with global", map[string]string{

"address": "https://api.myforgejoinstance.com",

"token": "some-token",

"name": "some-name",

"labels": "some-labels",

"global": "true",

"metric_path": "path",

}, false, "", true},

}

cobak78 requested a review from a team as a code owner January 22, 2025 14:40

cobak78 force-pushed the forgejo-runner-scaler branch from 03fd3a1 to 39fd60d Compare January 22, 2025 14:43

semgrep-app bot reviewed Jan 22, 2025

View reviewed changes

pkg/scalers/forgejo_runner_scaler_test.go Show resolved Hide resolved

semgrep-app bot reviewed Jan 22, 2025

View reviewed changes

pkg/scalers/forgejo_runner_scaler.go Outdated Show resolved Hide resolved

semgrep-app bot reviewed Jan 22, 2025

View reviewed changes

pkg/scalers/forgejo_runner_scaler_test.go Outdated Show resolved Hide resolved

semgrep-app bot reviewed Jan 22, 2025

View reviewed changes

pkg/scalers/forgejo_runner_scaler_test.go Show resolved Hide resolved

cobak78 force-pushed the forgejo-runner-scaler branch from 39fd60d to a7366bc Compare January 22, 2025 14:52

jaime merino added 2 commits January 23, 2025 12:41

new forgejo scaler with tests

998fbf2

Signed-off-by: jaime Merino <[email protected]>

fix lint errors

f13dbc7

Signed-off-by: jaime merino <[email protected]ª>

cobak78 force-pushed the forgejo-runner-scaler branch from 5b9d520 to f13dbc7 Compare January 23, 2025 11:41

order scalers alphabetically

54b7e5d

Signed-off-by: jaime merino <[email protected]ª>

JorTurFer reviewed Jan 24, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

new forgejo scaler #6495

new forgejo scaler #6495

cobak78 commented Jan 22, 2025

JorTurFer left a comment •

edited

Loading

JorTurFer Jan 24, 2025

JorTurFer Jan 24, 2025

JorTurFer Jan 24, 2025

JorTurFer Jan 24, 2025

JorTurFer Jan 24, 2025

JorTurFer Jan 24, 2025

JorTurFer Jan 24, 2025

JorTurFer Jan 24, 2025

JorTurFer Jan 24, 2025

JorTurFer Jan 24, 2025

		orgJobMetricPath = "/api/v1/orgs/%s/actions/runners/jobs" // /api/v1/orgs/{org}/actions/runners/jobs
		repoJobMetricPath = "/api/v1/repos/%s/%s/actions/runners/jobs" // {address}/api/v1/repos/{owner}/{repo}/actions/runners/jobs

	UserJobMetricPath = "/api/v1/user/actions/runners/jobs"
	userJobMetricPath = "/api/v1/user/actions/runners/jobs"


		metric := GenerateMetricInMili(metricName, float64(len(jobList.Jobs)))

		metric.Value.Add(resource.Quantity{})

new forgejo scaler #6495

Are you sure you want to change the base?

new forgejo scaler #6495

Conversation

cobak78 commented Jan 22, 2025

Checklist

JorTurFer left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JorTurFer left a comment •

edited

Loading