Skip to content

Commit

Permalink
fix: remove unnecessary github api call
Browse files Browse the repository at this point in the history
There are only a few cases, where we get a job information from github
where the runner name is not set.

For all this cases we do not need to check github API at all because
these jobs are never ever get scheduled to a runner:

job.Action is:

* queued:
  a queued job is just queued and not scheduled to a runner so we do
  not get a runner name from the GH API
* completed:
  when conclusion=cancelled|failure github never scheduled the job to a
  runner and with that we do not get a runner name from the GH API

Signed-off-by: Mario Constanti <mario.constanti@mercedes-benz.com>
  • Loading branch information
bavarianbidi committed Jun 5, 2024
1 parent 02ff74b commit dc74c45
Showing 1 changed file with 1 addition and 81 deletions.
82 changes: 1 addition & 81 deletions runner/pool/pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ import (
"github.com/cloudbase/garm-provider-common/util"
"github.com/cloudbase/garm/auth"
dbCommon "github.com/cloudbase/garm/database/common"
"github.com/cloudbase/garm/metrics"
"github.com/cloudbase/garm/params"
"github.com/cloudbase/garm/runner/common"
garmUtil "github.com/cloudbase/garm/util"
Expand Down Expand Up @@ -892,40 +891,6 @@ func (r *basePoolManager) addInstanceToProvider(instance params.Instance) error
return nil
}

func (r *basePoolManager) getRunnerDetailsFromJob(job params.WorkflowJob) (params.RunnerInfo, error) {
runnerInfo := params.RunnerInfo{
Name: job.WorkflowJob.RunnerName,
Labels: job.WorkflowJob.Labels,
}

var err error
if job.WorkflowJob.RunnerName == "" {
if job.WorkflowJob.Conclusion == "skipped" || job.WorkflowJob.Conclusion == "cancelled" {
// job was skipped or cancelled before a runner was allocated. No point in continuing.
return params.RunnerInfo{}, fmt.Errorf("job %d was skipped or cancelled before a runner was allocated: %w", job.WorkflowJob.ID, runnerErrors.ErrNotFound)
}
// Runner name was not set in WorkflowJob by github. We can still attempt to
// fetch the info we need, using the workflow run ID, from the API.
slog.InfoContext(
r.ctx, "runner name not found in workflow job, attempting to fetch from API",
"job_id", job.WorkflowJob.ID)
runnerInfo, err = r.GetRunnerInfoFromWorkflow(job)
if err != nil {
return params.RunnerInfo{}, errors.Wrap(err, "fetching runner name from API")
}
}

_, err = r.store.GetInstanceByName(context.Background(), runnerInfo.Name)
if err != nil {
slog.With(slog.Any("error", err)).ErrorContext(
r.ctx, "could not find runner details",
"runner_name", util.SanitizeLogEntry(runnerInfo.Name))
return params.RunnerInfo{}, errors.Wrap(err, "fetching runner details")
}

return runnerInfo, nil
}

// paramsWorkflowJobToParamsJob returns a params.Job from a params.WorkflowJob, and aditionally determines
// if the runner belongs to this pool or not. It will always return a valid params.Job, even if it errs out.
// This allows us to still update the job in the database, even if we determined that it wasn't necessarily meant
Expand Down Expand Up @@ -955,30 +920,14 @@ func (r *basePoolManager) paramsWorkflowJobToParamsJob(job params.WorkflowJob) (
CompletedAt: job.WorkflowJob.CompletedAt,
Name: job.WorkflowJob.Name,
GithubRunnerID: job.WorkflowJob.RunnerID,
RunnerName: job.WorkflowJob.RunnerName,
RunnerGroupID: job.WorkflowJob.RunnerGroupID,
RunnerGroupName: job.WorkflowJob.RunnerGroupName,
RepositoryName: job.Repository.Name,
RepositoryOwner: job.Repository.Owner.Login,
Labels: job.WorkflowJob.Labels,
}

runnerName := job.WorkflowJob.RunnerName
if job.Action != "queued" && runnerName == "" {
if job.WorkflowJob.Conclusion != "skipped" && job.WorkflowJob.Conclusion != "cancelled" {
// Runner name was not set in WorkflowJob by github. We can still attempt to fetch the info we need,
// using the workflow run ID, from the API.
// We may still get no runner name. In situations such as jobs being cancelled before a runner had the chance
// to pick up the job, the runner name is not available from the API.
runnerInfo, err := r.getRunnerDetailsFromJob(job)
if err != nil && !errors.Is(err, runnerErrors.ErrNotFound) {
return jobParams, errors.Wrap(err, "fetching runner details")
}
runnerName = runnerInfo.Name
}
}

jobParams.RunnerName = runnerName

switch r.entity.EntityType {
case params.GithubEntityTypeEnterprise:
jobParams.EnterpriseID = &asUUID
Expand Down Expand Up @@ -1968,35 +1917,6 @@ func (r *basePoolManager) ValidateOwner(job params.WorkflowJob) error {
return nil
}

func (r *basePoolManager) GetRunnerInfoFromWorkflow(job params.WorkflowJob) (params.RunnerInfo, error) {
if err := r.ValidateOwner(job); err != nil {
return params.RunnerInfo{}, errors.Wrap(err, "validating owner")
}
metrics.GithubOperationCount.WithLabelValues(
"GetWorkflowJobByID", // label: operation
r.entity.LabelScope(), // label: scope
).Inc()
workflow, ghResp, err := r.ghcli.GetWorkflowJobByID(r.ctx, job.Repository.Owner.Login, job.Repository.Name, job.WorkflowJob.ID)
if err != nil {
metrics.GithubOperationFailedCount.WithLabelValues(
"GetWorkflowJobByID", // label: operation
r.entity.LabelScope(), // label: scope
).Inc()
if ghResp != nil && ghResp.StatusCode == http.StatusUnauthorized {
return params.RunnerInfo{}, errors.Wrap(runnerErrors.ErrUnauthorized, "fetching workflow info")
}
return params.RunnerInfo{}, errors.Wrap(err, "fetching workflow info")
}

if workflow.RunnerName != nil {
return params.RunnerInfo{
Name: *workflow.RunnerName,
Labels: workflow.Labels,
}, nil
}
return params.RunnerInfo{}, fmt.Errorf("failed to find runner name from workflow")
}

func (r *basePoolManager) GithubRunnerRegistrationToken() (string, error) {
tk, ghResp, err := r.ghcli.CreateEntityRegistrationToken(r.ctx)
if err != nil {
Expand Down

0 comments on commit dc74c45

Please sign in to comment.