feat(ws): add spec.podTemplate.volumes.secrets[] to Workspace

[thesuperzapper]

related #239

This PR adds spec.podTemplate.volumes.secrets[] to the Workspace CRD, which lets users can mount secrets as volumes to their workspaces.

The Workspace CRD now has fields like this:

spec:
  podTemplate:
    volumes:
      secrets:
        - secretName: "workspace-secret"
          mountPath: "/secrets/my-secret"
          defaultMode: 420 # same as 0644 in octal

Because errors relating to missing secrets (which cause the pod to get stuck in "pending" state) are only surfaced through Pod events, we now extract Pod events when determining the Workspace state and stateMessage.

Additionally, because those events might not exist when we first reconcile the Workspace (and none of the current WS reconcile triggers would cause a reconciliation when a new event is raised), we simply always requeue every 15 seconds when a Workspace's Pod is in the Pending state (and hopefully pick up a Warning even which will break us out of this loop).

[google-oss-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from thesuperzapper. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[vdksystem]

@thesuperzapper, why don't we check if secret exists as part of reconciliation process? Then we can set ConfigError status or similar right away.
Or may be we have validation webhook which will prevent user from creating workspace with wrong config.
Also I wold add backoff to delayed reconciliation.

[thesuperzapper]

@thesuperzapper, why don't we check if secret exists as part of reconciliation process? Then we can set ConfigError status or similar right away. Or may be we have validation webhook which will prevent user from creating workspace with wrong config.

@vdksystem I think its safer to create the pod and see if it succeeds (e.g. eventual consistency). There are also some strange situations where a secret is only created when a pod tries to mount it.

I also want to avoid the validation webhook rejecting "potentially" valid configs (especially because a secret could be deleted after the Workspace, which would lead to an "update" being rejected on a previously acceptable manifest).

Also I wold add backoff to delayed reconciliation.

I think the only sensible way to achieve this would be to make the reconciliation loop "fail" in the case that we have a pending Pod with no events (that is, return non-nill err, but still update the Workspace state to Pending before doing that).

This would kick us into the exponential failure backoff of controller-runtime (which is probably way to aggressive, as discussed in #245). However, this would also be quite noisy in the controller logs, because each of the failed reconciles will show as an error even though we are really "waiting" for Pod events to be created.

Another approach is not returning an error, but storing a "retry count" in the status of the Workspace. However, this will cause a whole bunch of updates to the Workspace resource during the retry (which will also trigger other resources to reconcile, e.g. the WorkspaceKind used by the Workspace).

Not sure which is "best", do you have any thoughts?

[vdksystem]

@thesuperzapper agree with eventual consistency. Optional - we can implement kinda soft check. Not sure if it make sense to adjust status, but at least emit warning event.

With retries, we can use creationTimestamp and implement backoff logic around time since created (e.g. retyInterval = 15s * minutes since created).

I think all of that would make sense only at scale, can be ignored for now.

[github-actions]

This pull request has been automatically marked as stale because it has not had recent activity.
It will be closed if no further activity occurs.
Thank you for your contributions.

Members may comment /lifecycle frozen to prevent this pull request from being marked as stale.

[github-actions]

This pull request has been automatically closed because it has not had recent activity.
You can reopen the PR if you want.

[thesuperzapper]

/reopen

[google-oss-prow]

@thesuperzapper: Reopened this PR.

In response to this:

/reopen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[yashpal2104]

Does this PR require help?