When launching the controller with multiple queue tags it doesn't pick up any job from the pipeline resulting in pipelines in stuck waiting for agent.

Version 0.7.0
Startup log: ( org value is masked )
2024-01-18T08:26:10.909Z INFO controller/controller.go:122 configuration loaded {"config": {"agent-token-secret": "dev-eks-buildkite-agent-stack-secrets", "debug": true, "image": "ghcr.io/buildkite/agent-stack-k8s/agent:0.7.0", "job-ttl": "1h0m0s", "max-in-flight": 0, "namespace": "buildkite", "org": "mytestorg", "profiler-address": "", "cluster-uuid": "", "tags": ["queue=dev-eks", "queue=dev-cat-eks"]}} 2024-01-18T08:26:11.011Z INFO monitor monitor/monitor.go:113 started {"org": "mytestorg"}

If you mount secrets into your container usingenvFrom or something and then have set -x on, you'll see the secrets unredacted in your build logs.

I thought Buildkite would redact them, but for some reason not.

I'd like to add custom hooks to the agent image without updating the controller pod. I notice that when I deploy with --set image=my-custom-image:latest, the controller gets redeployed with my image. This was unexpected based on the help text.

--image string The image to use for the Buildkite agent (default "ghcr.io/buildkite/agent-k8s:latest")

Is that behavior intentional? And is there a recommended approach for customizing the agent images?

Right now we're having the checkout container copy the ~/.ssh directory and set permissions on it, taking advantage of the fact that it previously didn't have a command phase.

The problem is that a common pattern for when we're using Alpine Linux containers is to set BUILDKITE_SHELL to /bin/sh, because Alpine doesn't come out of the box with bash. With the way that the environment gets set in each of these containers, this means that BUILDKITE_SHELL then gets set to that on the checkout container, resulting in this weird error of

/bin/sh: can't open 'trap 'kill -- $' INT TERM QUIT; cp -r ~/.ssh /workspace/.ssh && chmod -R 777 /workspace': No such file or directory

One way of solving this would be to just not set BUILDKITE_SHELL on the checkout container, we already do some kind of deny-listing like this with other parts of the environment.

Another way would be to find the minimum subset of variables that the system containers need (agent, checkout, artifact upload), and only give the entire environment to the command/sidecar containers.

A third way we could solve this would be to move this logic into the agent itself, rather than using the command step like this.

A way that users can work around this is to set the env only on the specific Alpine container, but I'm thinking that the current default when you set env at the step level might be surprising behavior for some.

Right now it's slightly confusing:

ghcr.io/buildkite/helm/agent-stack-k8s
ghcr.io/buildkite/agent-k8s
ghcr.io/buildkite/agent-stack-k8s

How about:

ghcr.io/buildkite/agent-stack-k8s/helm
ghcr.io/buildkite/agent-stack-k8s/agent
ghcr.io/buildkite/agent-stack-k8s/controller

In #107 I'm disabling it since I'm hitting weird issues like https://buildkite.com/buildkite-kubernetes-stack/kubernetes-agent-stack/builds/294#_

ERRO Running error: context loading failed: no go files to analyze

 
I have no idea why that's happening but can't be bothered to troubleshoot that right now. Linter is not providing a ton of value and is pretty slow because of the lack of caching.

Expose this over a local port for troubleshooting. I've seen the controller seemingly get stuck and the logs not reveal anything useful.

If we give the agent access via rbac to its own pod, it can use the Kubernetes API to query the status of other containers in its pod. This would let us log container configuration errors, like missing secrets/configmaps, or image pull errors.

This might mean the agent takes a dependency on client-go.

0.5.1 does not support overriding token - please release the new version!

I am trying to deploy this helm chart in my org spinnaker, it requires org-specific labels, but the helm chart yaml config did not allow user-input labels. It's better to allow user-input labels into helm chart config yaml.

We're using a shared informer to calculate the number of jobs in flight and keep it below a certain threshold (max-in-flight).

The field we're looking at to distinguish in-progress jobs from completed jobs is currently job.Status.Active, which results in a bug that when jobs are first created and haven't yet been reconciled by the Job controller, active is 0. #88 fixes this.

The additional problem is that even with this fix, it's possible for the monitor thread to get blocked trying to push another job onto the channel we're using as a queue between the monitor and the scheduler.

• monitor: get 2 jobs from api server
• monitor: check current-in-flight, find 0
• monitor: push job 1 on queue
• scheduler: pull job 1 off queue
• monitor: check current-in-flight, find 0
• monitor: push job 2 on queue
• scheduler: create job 1
• scheduler: pull job 2 off queue
• scheduler create job 2

I think in order to fix this problem we're going to have to slightly rethink how we do max-in-flight calculations. I think we need to do some number tracking on our own, and not just push it all to the client. We can use the informer to be notified of job completions, but we can keep track of job creations ourselves. I think it'd likely be simplest if we move the max-in-flight calculation to the scheduler completely, and let the monitor just be limited by the bounded channel.

We should disassociate these test pipelines from the repo so they don't show up in commit status

I've noticed that GKE node pool autoscaler will evict jobs in progress if autoscaling policy is set to optimize-utilization. Default setting (balanced) doesn't have that problem.

Based on GKE autoscaling policy docs this can be prevented by adding cluster-autoscaler.kubernetes.io/safe-to-evict: "false" annotation.

This is not a GKE specific thing:
https://kubernetes.io/docs/reference/labels-annotations-taints/#cluster-autoscaler-kubernetes-io-safe-to-evict

same as nodeSelector parameter, we need a parameter to specify tolerations when deploying the agent-stack-k8s pods.

controller (from deployment) uses the custom secret passed via helm - but created job does not pick it up.

I believe this is because configmap is not storing the custom info at https://github.com/buildkite/agent-stack-k8s/blob/v0.6.0/charts/agent-stack-k8s/templates/config.yaml.tpl

This breaks one of the more common things that are going to be used with this plugin: Kaniko - only debug image has a shell, regular one doesn't.

This should at the very least be documented.

steps:
  - label: build image
    artifact_paths:
      - "myfile"
    agents:
      queue: kubernetes
    plugins:
      - kubernetes:
          podSpec:
            containers:
              - image: alpine:latest
                command: [echo]
                args:
                - false
                - exit 1

On completion, the entire pod is torn down, rather than exiting its own container and continuing to go into the artifact container
That means that no artifacts are uploaded

Right now we're building the agent on every commit, but we could be smarter and only build it if it's changed.

I think we might want to tag the image with the commit of the agent repo, so that then we can find the already-built image for it in the case that it hasn't changed.

The current agent approach would be challenging for large repositories due to the time to clone. Can we create a mechanism for caching the repo within the cluster so that jobs using that repository will start more quickly?

Check solutions in the existing k8s ecosystem

Should be really straightforward, just use --platform here

https://ko.build/reference/ko_build/

Via prometheus/client_golang probably.

• Jobs processed
• GraphQL query latency
• API error rates
• and more?

Right now we only look at the first 100 scheduled builds, which is a pretty good simple approach, since as the builds are started we should see new ones appear on the next time we poll. It seems like it might be more performant though to iterate through the pages instead of waiting for the next loop in order to schedule all possible builds.

Seems like helm values schema check doesn't let me use agentStackSecret instead of agentToken and graphqlToken.

Error: UPGRADE FAILED: values don't meet the specifications of the schema(s) in the following chart(s):
agent-stack-k8s:
- agentToken: String length must be greater than or equal to 1
- graphqlToken: String length must be greater than or equal to 1

Workaround: pass any value to pass the values schema validation.

We don't currently allow users to use any plugins other than our own. It might not be that difficult, we just need to remove our plugin from the plugin data and pass it through to the agent. This one would be a great one to have working: https://github.com/buildkite-plugins/test-collector-buildkite-plugin

Created from: #45
cc @titilambert

In testing agent-stack-k8s, some of my podSpecs I wrote ended up being incorrectly formatted. When this happens, agent-stack-k8s tries to replace the container with a build failure job which just echoes the underlying error to the console, as defined in https://github.com/buildkite/agent-stack-k8s/blob/main/internal/controller/scheduler/scheduler.go#L433-L447.

However, when using gitEnvFrom for SSH credentials to checkout git, the underlying error gets masked, and instead the console just shows failure to checkout the Git repo (failing after 3 retries), and the underlying error is never printed. This is because the checkout container (nor any other container) do not get the gitEnvFrom attached to it when there a build failure. Thus checkout fails and the container echoing the error never prints.

The only workaround to this is to run the job, then identify and examine the pod it creates while it is in flight, and examine the pod manifest looking for the error text in the BUILDKITE_COMMAND environment variable. This is obviously not optimal.

We mount the shared container workspace to /workspace, but that might cause collisions for users. We should choose something less generic (/buildkite?) and offer a way to override it.

• allow disabling checkout
• allow setting checkout-specific env vars (BUILDKITE_GIT_CLONE_FLAGS, BUILDKITE_GIT_FETCH_FLAGS) - you'd normally do it using a plugin but since this plugins also does checkout you can't

cc @jmcshane

Pulled out of #104 (comment)

We sometimes seem jobs that get stuck for some reason (misconfiguration, etc) that causes them to get into a unrecoverable state. If a user hits "cancel" on the Buildkite UI, we should notice this and delete the job from Kubernetes.

Most of the time the agent will see the cancellation and do cleanup, but there are still times when the agent has failed or hasn't started that the job can be orphaned.

We could do this with a background task that periodically fetches builds we are tracking and checks their status on the Buildkite API.

#90 introduced a different way of managing max-in-flight. We saw that the old way was buggy and would allow more jobs in flight than the user specified, because there's lag in between the job being created and the cached informer learning about it.

The new rate limiter has a couple bugs I noticed, but the worst is a deadlock. This is the stack dump, basically it's stuck waiting on a completion, holding the mutex, while at the same time it's trying to add a new job.

Not sure I can totally reason about how completions is at capacity, but at least one reason is that we're not handling OnAdd, which means that when the controller starts it ignores any jobs that are already running. We then get completions for those jobs when they finish/delete, meaning we might have more completions than we have jobs in flight.

Another problem is the check for job.Status.CompletionTime, you only get that if a job "completes", aka finishes successfully.

The completion time is only set when the job finishes successfully
It seems there's no single way to find if a job just finished, we need to look at multiple fields, or multiple conditions, to get both success or failure.

Another problem is that if the limiter sends a job on to the scheduler but the scheduler errors when trying to create it, it will never get removed from the in-flight map, since we have no way of learning about creating failures.

Basically, a bunch of problems. I think the right solution is to

• Change the API to just be a regular middleware pattern, like http.Handler. Gets rid of the nonsense of channels on the external API, and allows errors to bubble back up through the Limiter
• Just do a len(inFlight) to check if we're at max-in-flight, and if so we can just drop the Job and let it be requeued later when we poll the API again. Get rid of the completions channel and its over complicated attempt to do this rate-limiting.
• Fix up the job completion logic, that part is pretty straightforward

#113 gives us better visibility into pod failures like invalid YAML or invalid Kubernetes objects

This issue is an umbrella issue for thinking about other failure states that user might be frustrated by:

• Insufficient capacity on the cluster, resource requirements that can't be met

For example, a container-0 job ran into this OOM so it has already exited:

 - containerID: containerd://868661c9da807af9428729518d1c95a52c1bb5efac68df8799cd6b24b475125c
    image: docker.io/library/golang:1.20-alpine
    imageID: docker.io/library/golang@sha256:59fc0dc542a38bb5b94cd1529e5f4663b4e7cc2f4a6c352b826dafe00d820031
    lastState: {}
    name: container-0
    ready: false
    restartCount: 0
    started: false
    state:
      terminated:
        containerID: containerd://868661c9da807af9428729518d1c95a52c1bb5efac68df8799cd6b24b475125c
        exitCode: 137
        finishedAt: "2023-07-14T10:40:18Z"
        reason: OOMKilled
        startedAt: "2023-07-14T10:31:49Z"

But in the buildkite UI it still appears running:

Maybe need a way for the controller to detect OOM events in the jobs to clean them up?

Currently, the agent-stack-k8s requires that the secret material for both agentToken and graphqlToken are provided to helm, for helm to templatize the data into a Secret object. This is incompatible with other secret management solutions like External Secrets Operator. In these solutions, you would want to tell helm where to look for the secret and such, and allow the operator to create the actual Secret object. And very specifically for us, doing so would imply committing those rendered files to git, as we leverage a workflow as outlined in Robbie's talk at Unblock a few year ago

Example

As an example for this workflow, you might have in your chart, that leverages agent-stack-k8s as a subchart, a template like

apiVersion: external-secrets.io/v1beta1
kind: ExternalSecret
metadata:
  name: template
spec:
  # ...
  target:
    name: my-secret
    creationPolicy: Owner
  data:
  - secretKey: agent-token
    remoteRef:
      key: /agent-token

The ESO will come in and fetch the secret from an external store (vault, AWS Secrets Manager) and create my-secret Secret from this template .

You could then reference it in a helm chart with

secret:
  create: false
  existingSecret: my-secret
  agentTokenKey: agent-token

Prior Art

This is effectively an ask to reproduce the work in buildkite/charts#96 into the new chart

Acceptance Criteria i.e. TLDR

Secrets for agentToken and graphqlToken can be specified via a combination of an externally created secret name, and a key within that Secret

Update: I now realize the issue I was experiencing was with the execution of another plugin, and not something agent-stack-k8s was doing.

Closing. Sorry for the trouble.

Kubernetes Environment: Amazon EKS
Runtime: containerd

Found tag v1.11.0, pulling from docker hub
Cannot connect to the Docker daemon at unix:///var/run/docker.sock. Is the docker daemon running?
Command failed with exit status: 1
Suppressing failure so pipeline can continue (if you do not want this behaviour, set fail_on_error to true)

agents:
  queue: kubernetes

steps:
  - label: "example"
    artifact_paths: ["golangci-lint.xml"]
    plugins:
      - kubernetes:
          gitEnvFrom:
            - secretRef: { name: agent-stack-k8s }
          podSpec:
            containers:
              - image: ghcr.io/buildkite/agent-stack-k8s/agent:latest
                command: [bash]
                args:
                  - -ec
                  - "'echo test >> golangci-lint.xml'"
      - bugcrowd/test-summary#v1.11.0:
          inputs:
            - label: golangci-lint
              artifact_path: artifacts/golangci-lint.xml
              type: checkstyle
          formatter:
            type: details
          run_without_docker: true

Toleration + node selector

Right now we just wait for sidecars to connect before running the main command container, but this means that there's still a race condition between the command and sidecar processes. You end up needing retries in your code to handle the fact that the sidecars may not be ready yet.

We can follow the Tekton approach of supporting container probes and waiting on them instead:
https://github.com/tektoncd/pipeline/blob/main/cmd/entrypoint/README.md#waiting-for-sidecars

Because there isn't a checkout step (it's actually a command step under the hood), any configured post checkout hooks won't execute because there isn't an official checkout

Saw this in rails-docker-parallel-example

Should we be setting this on our agents?

All job and pod names are configured to be buildkite-%s

We have multiple environments, so it'll be easier to identify where jobs are run if this value was configurable.

While running a lot of parallel jobs I have some jobs that doesn't start at all showing up in the UI as "Canceled". I managed to reduce the number of canceled jobs by:

1. Increasing registry QPS on the kubelet
2. Moving the docker image to a private registry
3. Pulling the image at the kubernetes nodes startup ( managed by karpenter on EKS )

Sometimes I still get Canceled jobs in the UI and the pods logs this:

2023-12-14 11:30:14 DEBUG  Loaded config agent_version=3.52.0 agent_build=6806
2023-12-14 11:30:14 DEBUG  Enabled experiment "kubernetes-exec" agent_version=3.52.0 agent_build=6806
# Using experimental Kubernetes support
🚨 Error: Failed to start kubernetes client: error connecting to kubernetes runner: dial unix /workspace/buildkite.sock: connect: no such file or directory

Describing the job it says:

Events:
  Type     Reason            Age    From            Message
  ----     ------            ----   ----            -------
  Normal   SuccessfulCreate  7m53s  job-controller  Created pod: buildkite-018c6813-a298-4984-8707-ad9a72dd306e-6mfjh
  Normal   SuccessfulDelete  3m28s  job-controller  Deleted pod: buildkite-018c6813-a298-4984-8707-ad9a72dd306e-6mfjh
  Warning  DeadlineExceeded  3m28s  job-controller  Job was active longer than specified deadline

I noticed that activeDeadlineSeconds is 1 and backoffLimit is 0. These settings should be configurable to allow the Pod to retry in case of image pull issues etc but I'm not sure if it's relevant for this problem.

If global environment variables are set in the top level pipeline file as an int, it'll cause a golang error and won't be processed

env:
  myenv: 1

steps:
  - label: build image
    agents:
      queue: kubernetes
    plugins:
      - kubernetes:
          podSpec:
            containers:
              - image: alpine:latest
                command: [echo]
                env:
                  - name: DOCKER_HOST
                     value: tcp://localhost:2375
                args:
                - "Hello, world!"

Also, the env variables should be merged, so any global envs should be merged into the local step envs

Kaniko image has entrypoint /kaniko/executor, see https://github.com/GoogleContainerTools/kaniko/blob/main/deploy/Dockerfile#L100

but:

- label: builder    
    plugins:
      - kubernetes:
          podSpec:
            containers:
              - image: gcr.io/kaniko-project/executor:debug
                args:
                  - "--context=/workspace/[...removed...]"
                  - "--dockerfile=Dockerfile"
                  - "--destination=my-image"

gets me:

trap 'kill -- $$' INT TERM QUIT; --context=/workspace/[...removed...] --dockerfile=Dockerfile --destination=my-image
/bin/sh: --context=/workspace/[...removed...]: not found

this works:

- label: builder    
    plugins:
      - kubernetes:
          podSpec:
            containers:
              - image: gcr.io/kaniko-project/executor:debug
                command: [/kaniko/executor]
                args:
                  - "--context=/workspace/[...removed...]"
                  - "--dockerfile=Dockerfile"
                  - "--destination=my-image"

Again, this should at the very least be documented.

Different containers have different default users, so we should make the cloned repo in the /workspace tempdir globally read/write so that user-containers don't encounter perms problems

If a user wants to use a plugin that requires auth to clone it, or if they want to do anything else that would require their ssh secret in a non-checkout container.

We should just pass the secret to all containers.

cc @titilambert

Changes to podSpec result in unpredictable checkout failures, example:

steps:
  - label: ':pipeline: Pipeline Setup'
    agents:
      queue: my-ci
    plugins:
      - kubernetes:
          gitEnvFrom:
            - secretRef:
                name: buildkite-agent-ssh
          podSpec:
            containers:
              - image: 'buildkite/agent:latest'
                command:
                  - buildkite-agent
                args:
                  - pipeline upload

Works perfectly - 100% success rate

Adding an extra container results in 100% failure rate on checkout stage - missing creds - before any of the containers in spec are started. I have managed to trigger this by modifications as small as an additional space between args

steps:
  - label: ':pipeline: Pipeline Setup'
    agents:
      queue: my-ci
    plugins:
      - kubernetes:
          gitEnvFrom:
            - secretRef:
                name: buildkite-agent-ssh
          podSpec:
            containers:
              - image: 'buildkite/agent:latest'
                command:
                  - buildkite-agent
                args:
                  - pipeline upload
              - image: 'buildkite/agent:latest'
                command:
                  - /bin/bash
                args:
                  - echo $${BUILDKITE_BRANCH}

Adding

env:
    GIT_SSH_COMMAND: ssh -vvv

yields

[...]
debug1: Connection established.
debug1: identity file /root/.ssh/id_rsa type 0
debug1: identity file /root/.ssh/id_rsa-cert type -1
[...]

for the first example

[...]
debug1: Connection established.
debug1: identity file /root/.ssh/id_rsa type -1
debug1: identity file /root/.ssh/id_rsa-cert type -1
[...]

for the second one

With

env:
    GIT_SSH_COMMAND: ssh -i /workspace/.ssh -vvv 

second example reports that /workspace/.ssh doesn't exist

With

env:
    BUILDKITE_GIT_CLONE_FLAGS: "--depth=1"

That flag is passed to git command in first example, but not in the second.

I just got an example where my step was green even if the plugin crashed because of a missing permission

The step should be red if there is this kind of issue

agent-stack-k8s allows us to set the resource requests/limits (e.g. cpu and memory) for the controller that launches pods, but does not provide a way to specify those requests/limits for the Buildkite agent pods it launches. Implicitly, you can provide resources for pods launched by the podSpec specified in a step, but not for the pods that the controller launches for checking out the Git repo or for monitoring the pods it launched.

This is especially needed if we are running the commands directly in the agent container (e.g. with no podSpec, just using a regular command step), where our commands could run some more cpu/memory intensive operations (in combination with our own custom agent containers with our own libraries installed). Proper requests/limits will allow us to combine agent-stack-k8s with an autoscaler like Karpenter to dynamically size our Buildkite compute appropriately.

Even better would be supporting different container-level resource limits/requests depending on if the agent is running the command block directly versus just using a podSpec and launching separate containers (where we can provide our own requests/limits). Even better on top of that would be having a concept of default requests/limits for containers specified in a podSpec.

Somewhere in the Build function in the scheduler, annotations that are passed in via the plugin configuration are being lost.

For example I have a configuration of:

name: "test pipeline"
agents:
   queue: kubernetes
plugins:
- kubernetes:
     metadata:
        annotations:
              example.com/annotation-test: "value"
     podSpec:
       containers:
       - name: app
          image: debian
          command: ["echo", "hello world"]

The resultant job will not have the annotation provided, but will have the buildkite build-url and job-url annotations

I think they're being overwritten somewhere in here:

A user asked if they could use https://github.com/jtblin/kube2iam, which relies on annotations on pods in order to assign them IAM roles. We currently don't expose any way to control metadata.