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

Run Windows node CSI Drivers as HostProcess containers #217

Open
26 of 30 tasks
mauriciopoppe opened this issue Sep 15, 2022 · 11 comments
Open
26 of 30 tasks

Run Windows node CSI Drivers as HostProcess containers #217

mauriciopoppe opened this issue Sep 15, 2022 · 11 comments
Assignees
Labels
lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.

Comments

@mauriciopoppe
Copy link
Member

mauriciopoppe commented Sep 15, 2022

The HostProcess container feature became beta in 1.23, we'd like to leverage it in CSI Drivers which will run as privileged jobs in the Windows host, there'll be more details about the transition steps in the design doc.

By making the CSI Driver a HostProcess pod we no longer need the binary in the client/server model (although we will still support it). There are some items for the maintenance of the current client/server model of CSI Proxy.

Items for the refactor of CSI Proxy to become a go library:

Items to use the new CSI Proxy library in a CSI Driver (PD CSI Driver):

/assign @mauriciopoppe

@mauriciopoppe mauriciopoppe changed the title Refactor CSI Proxy to become a go library in favor of the client/server model Design doc: Refactor CSI Proxy to become a go library in favor of the client/server model Sep 15, 2022
@mauriciopoppe mauriciopoppe changed the title Design doc: Refactor CSI Proxy to become a go library in favor of the client/server model Transition of CSI Drivers to become HostProcess containers Oct 6, 2022
@mauriciopoppe mauriciopoppe changed the title Transition of CSI Drivers to become HostProcess containers Transition of CSI Drivers to become HostProcess Pods Oct 6, 2022
@msau42
Copy link
Contributor

msau42 commented Oct 6, 2022

@mauriciopoppe if the work to migrate to hostprocess has not merged yet, can we do a release now with the existing workflow and then create a branch?

@mauriciopoppe
Copy link
Member Author

@msau42 yeah we can do that, I wanted to try to release in the v1.x branch so that if we need to have a new release we would have everything set up (if that's needed although that might not happen). I'll proceed with the release in the master branch.

@mauriciopoppe
Copy link
Member Author

Updates about the ongoing work:

@alexander-ding
Copy link
Contributor

Following up on @mauriciopoppe 's points:

  • We are drafting's a release blog post for CIS Proxy v2. Any feedback would be greatly appreiated.

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle stale
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Mar 15, 2023
@mauriciopoppe
Copy link
Member Author

/remove-lifecycle stale
/lifecycle frozen

@k8s-ci-robot k8s-ci-robot added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Apr 7, 2023
@davhdavh
Copy link

davhdavh commented Jul 6, 2023

For anyone interested, there is no reason to wait for v2 to use host process...

This is all the yaml you need to get it working.

apiVersion: apps/v1
kind: DaemonSet
metadata:
  labels:
    k8s-app: csi-proxy
  name: csi-proxy
  namespace: kube-system
spec:
  selector:
    matchLabels:
      k8s-app: csi-proxy
  template:
    metadata:
      labels:
        k8s-app: csi-proxy
    spec:
      nodeSelector:
        "kubernetes.io/os": windows
      securityContext:
        windowsOptions:
          hostProcess: true
          runAsUserName: "NT AUTHORITY\\SYSTEM"
      hostNetwork: true
      containers:
        - name: csi-proxy
          image: ghcr.io/kubernetes-sigs/sig-windows/csi-proxy:v1.1.2

@alexander-ding
Copy link
Contributor

alexander-ding commented Jul 6, 2023

For anyone interested, there is no reason to wait for v2 to use host process...

This is all the yaml you need to get it working.


apiVersion: apps/v1

kind: DaemonSet

metadata:

  labels:

    k8s-app: csi-proxy

  name: csi-proxy

  namespace: kube-system

spec:

  selector:

    matchLabels:

      k8s-app: csi-proxy

  template:

    metadata:

      labels:

        k8s-app: csi-proxy

    spec:

      nodeSelector:

        "kubernetes.io/os": windows

      securityContext:

        windowsOptions:

          hostProcess: true

          runAsUserName: "NT AUTHORITY\\SYSTEM"

      hostNetwork: true

      containers:

        - name: csi-proxy

          image: ghcr.io/kubernetes-sigs/sig-windows/csi-proxy:v1.1.2

Hi there! Thanks for your interest in the project :)

It's been a bit since I've worked on this, but I'm fairly certain this shouldn't work, or at least this doesn't do what we are intending to do.

The CSI Proxy v1 container is a proxy server that relays client API calls to a privileged binary directly running on Windows machines (outside of Kubernetes). The point of v2 is to completely eliminate the need for the separate privileged binary. It is true that HostProcess containers (HPCs) are available already, but just running the v1 proxy server container as an HPC doesn't actually eliminate the need for the separate binary. Also, because HPCs don't support named pipes or unix sockets, the proxy server would likely fail to connect to binary completely.

@davhdavh
Copy link

davhdavh commented Jul 9, 2023

ghcr.io/kubernetes-sigs/sig-windows/csi-proxy:v1.1.2 includes the required binary, hostProcess: true gives it privilege.

Not sure why the HPC would need to support pipes or sockets specifically, it just runs like a normal process in the host and thus native support.

I'm fairly certain this shouldn't work

It works just fine; with smb atleast

The point of v2 is to completely eliminate the need for the separate privileged binary

Sure, a totally valid goal, but as it isn't available today, I gave a solution for the thing is available today.

@mauriciopoppe
Copy link
Member Author

What you propose was already implemented by the Windows team a while ago in https://github.com/kubernetes-sigs/sig-windows-tools/blob/master/hostprocess/csi-proxy/README.md, while it's running only CSI Proxy as a HostProcess Pod our goal is to leverage the HostProcessContainers feature to improve other parts of the entire solution (e.g. maintainability, parity between Linux & Windows implementations).

So while you're using HPC your CSI Driver deployment is different between Linux/Windows still, this is the problem we'd like to solve.

@alexander-ding
Copy link
Contributor

ghcr.io/kubernetes-sigs/sig-windows/csi-proxy:v1.1.2 includes the required binary, hostProcess: true gives it privilege.

Not sure why the HPC would need to support pipes or sockets specifically, it just runs like a normal process in the host and thus native support.

I'm fairly certain this shouldn't work

It works just fine; with smb atleast

The point of v2 is to completely eliminate the need for the separate privileged binary

Sure, a totally valid goal, but as it isn't available today, I gave a solution for the thing is available today.

Yeah, you're totally right. For some reason I thought you were suggesting running the node plugins/registrars as HPCs, instead of running the privileged binary as an HPC. My bad on this.

mauriciopoppe added a commit to mauriciopoppe/csi-proxy that referenced this issue Jul 20, 2023
4133d1d Merge pull request kubernetes-csi#226 from msau42/cloudbuild
8d519d2 Pin buildkit to v0.10.6 to workaround v0.11 bug with docker manifest
6e04a03 Merge pull request kubernetes-csi#224 from msau42/cloudbuild
26fdfff Update cloudbuild image
6613c39 Merge pull request kubernetes-csi#223 from sunnylovestiramisu/update
0e7ae99 Update k8s image repo url
77e47cc Merge pull request kubernetes-csi#222 from xinydev/fix-dep-version
155854b Fix dep version mismatch
8f83905 Merge pull request kubernetes-csi#221 from sunnylovestiramisu/go-update
1d3f94d Update go version to 1.20 to match k/k v1.27
e322ce5 Merge pull request kubernetes-csi#220 from andyzhangx/fix-golint-error
b74a512 test: fix golint error
aa61bfd Merge pull request kubernetes-csi#218 from xing-yang/update_csi_driver
7563d19 Update CSI_PROW_DRIVER_VERSION to v1.11.0
a2171be Merge pull request kubernetes-csi#216 from msau42/process
cb98782 Merge pull request kubernetes-csi#217 from msau42/owners
a11216e add new reviewers and remove inactive reviewers
dd98675 Add step for checking builds
b66c082 Merge pull request kubernetes-csi#214 from pohly/junit-fixes
b9b6763 filter-junit.go: fix loss of testcases when parsing Ginkgo v2 JUnit
d427783 filter-junit.go: preserve system error log
38e1146 prow.sh: publish individual JUnit files as separate artifacts

git-subtree-dir: release-tools
git-subtree-split: 4133d1df083eaa65bdeddd0530d54278529c7a60
marosset added a commit to marosset/csi-proxy that referenced this issue Jul 25, 2023
c10b678 Merge pull request kubernetes-csi#227 from coulof/check-sidecar-supported-versions
b055535 Header
bd0a10b typo
c39d73c Add comments
f6491af Script to verify EOL sidecar version
4133d1d Merge pull request kubernetes-csi#226 from msau42/cloudbuild
8d519d2 Pin buildkit to v0.10.6 to workaround v0.11 bug with docker manifest
6e04a03 Merge pull request kubernetes-csi#224 from msau42/cloudbuild
26fdfff Update cloudbuild image
6613c39 Merge pull request kubernetes-csi#223 from sunnylovestiramisu/update
0e7ae99 Update k8s image repo url
77e47cc Merge pull request kubernetes-csi#222 from xinydev/fix-dep-version
155854b Fix dep version mismatch
8f83905 Merge pull request kubernetes-csi#221 from sunnylovestiramisu/go-update
1d3f94d Update go version to 1.20 to match k/k v1.27
e322ce5 Merge pull request kubernetes-csi#220 from andyzhangx/fix-golint-error
b74a512 test: fix golint error
aa61bfd Merge pull request kubernetes-csi#218 from xing-yang/update_csi_driver
7563d19 Update CSI_PROW_DRIVER_VERSION to v1.11.0
a2171be Merge pull request kubernetes-csi#216 from msau42/process
cb98782 Merge pull request kubernetes-csi#217 from msau42/owners
a11216e add new reviewers and remove inactive reviewers
dd98675 Add step for checking builds
b66c082 Merge pull request kubernetes-csi#214 from pohly/junit-fixes
b9b6763 filter-junit.go: fix loss of testcases when parsing Ginkgo v2 JUnit
d427783 filter-junit.go: preserve system error log
38e1146 prow.sh: publish individual JUnit files as separate artifacts

git-subtree-dir: release-tools
git-subtree-split: c10b67804e07a324fe33595040afd13f020ee000
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.
Projects
None yet
Development

No branches or pull requests

6 participants