Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Merged by Bors] - Secret-aware pod scheduling #125

Closed
wants to merge 10 commits into from

Conversation

nightkr
Copy link
Member

@nightkr nightkr commented May 6, 2022

Description

Fixes #57

This requires pod volume definitions to switch from using the csi volume type (which are fully contained within the Kubelet's Pod lifecycle) to the ephemeral volume type (which is reified into a full PersistentVolumeClaim by the scheduler). For example:

---
apiVersion: v1
kind: Pod
metadata:
  name: example-secret-consumer
spec:
  volumes:
    - name: tls
      csi:
        driver: secrets.stackable.tech
        volumeAttributes:
          secrets.stackable.tech/class: tls
          secrets.stackable.tech/scope: node,pod,service=secret-consumer-nginx
  containers:
    - name: ubuntu
      image: ubuntu
      stdin: true
      tty: true
      volumeMounts:
        - name: tls
          mountPath: /tls

should be changed into:

---
apiVersion: v1
kind: Pod
metadata:
  name: example-secret-consumer
spec:
  volumes:
    - name: tls
      ephemeral:
        volumeClaimTemplate:
          metadata:
            annotations:
              secrets.stackable.tech/class: secret
              secrets.stackable.tech/scope: node,pod,service=secret-consumer-nginx
          spec:
            storageClassName: secrets.stackable.tech
            accessModes:
              - ReadWriteOnce
            resources:
              requests:
                storage: "1"
  containers:
    - name: ubuntu
      image: ubuntu
      stdin: true
      tty: true
      volumeMounts:
        - name: tls
          mountPath: /tls

csi volumes still work for now, but won't be able to take advantage of secret-aware scheduling. We will probably want to remove it after all operators and documentation have been migrated.

Review Checklist

  • Code contains useful comments
  • (Integration-)Test cases added (or not applicable)
  • Documentation added (or not applicable)
  • Changelog updated (or not applicable)
  • Cargo.toml only contains references to git tags (not specific commits or branches)
  • Helm chart can be installed and deployed operator works (or not applicable)

Once the review is done, comment bors r+ (or bors merge) to merge. Further information

@nightkr nightkr self-assigned this May 6, 2022
@nightkr nightkr marked this pull request as ready for review May 6, 2022 12:02
@nightkr nightkr changed the title Configure CSI topologies to nudge pods onto nodes with available secrets Secret-aware pod scheduling May 6, 2022
@nightkr nightkr requested review from soenkeliebau and a team May 6, 2022 12:09
nightkr added a commit to stackabletech/operator-rs that referenced this pull request May 6, 2022
Copy link
Member

@sbernauer sbernauer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM overall % comments

rust/operator-binary/src/backend/dynamic.rs Show resolved Hide resolved
rust/operator-binary/src/csi_server/identity.rs Outdated Show resolved Hide resolved
rust/operator-binary/src/csi_server/node.rs Show resolved Hide resolved
rustfmt.toml Show resolved Hide resolved
Co-authored-by: Sebastian Bernauer <sebastian.bernauer@stackable.de>
CHANGELOG.md Outdated Show resolved Hide resolved
@nightkr nightkr requested a review from soenkeliebau May 10, 2022 08:18
@nightkr
Copy link
Member Author

nightkr commented May 11, 2022

bors r+

bors bot pushed a commit that referenced this pull request May 11, 2022
## Description

Fixes #57

This requires pod volume definitions to switch from using the `csi` volume type (which are fully contained within the Kubelet's Pod lifecycle) to the `ephemeral` volume type (which is reified into a full `PersistentVolumeClaim` by the scheduler). For example:

```yaml
---
apiVersion: v1
kind: Pod
metadata:
  name: example-secret-consumer
spec:
  volumes:
    - name: tls
      csi:
        driver: secrets.stackable.tech
        volumeAttributes:
          secrets.stackable.tech/class: tls
          secrets.stackable.tech/scope: node,pod,service=secret-consumer-nginx
  containers:
    - name: ubuntu
      image: ubuntu
      stdin: true
      tty: true
      volumeMounts:
        - name: tls
          mountPath: /tls
```

should be changed into:

```yaml
---
apiVersion: v1
kind: Pod
metadata:
  name: example-secret-consumer
spec:
  volumes:
    - name: tls
      ephemeral:
        volumeClaimTemplate:
          metadata:
            annotations:
              secrets.stackable.tech/class: secret
              secrets.stackable.tech/scope: node,pod,service=secret-consumer-nginx
          spec:
            storageClassName: secrets.stackable.tech
            accessModes:
              - ReadWriteOnce
            resources:
              requests:
                storage: "1"
  containers:
    - name: ubuntu
      image: ubuntu
      stdin: true
      tty: true
      volumeMounts:
        - name: tls
          mountPath: /tls
```

`csi` volumes still work for now, but won't be able to take advantage of secret-aware scheduling. We will probably want to remove it after all operators and documentation have been migrated.
@bors
Copy link
Contributor

bors bot commented May 11, 2022

Pull request successfully merged into main.

Build succeeded:

@bors bors bot changed the title Secret-aware pod scheduling [Merged by Bors] - Secret-aware pod scheduling May 11, 2022
@bors bors bot closed this May 11, 2022
@bors bors bot deleted the feature/csi-topology branch May 11, 2022 09:13
bors bot pushed a commit to stackabletech/operator-templating that referenced this pull request May 13, 2022
Merge this after stackabletech/secret-operator#125 get's merged and didn't case any problems
I had a quick chat with @soenkeliebau and we agreed to give it a try
bors bot pushed a commit to stackabletech/operator-rs that referenced this pull request May 13, 2022

## Description

Requires stackabletech/secret-operator#125, see that PR for motivation



Co-authored-by: Teo Klestrup Röijezon <teo.roijezon@stackable.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Secret topology support
3 participants