PWX-31562 : Operator's Windows support #1176

nikita-bhatia · 2023-07-31T10:10:33Z

Changes done :

Following node affinity is added to STC so that all the existing pods are scheduled on linux node only.

   requiredDuringSchedulingIgnoredDuringExecution:
     nodeSelectorTerms:
       - matchExpressions:
           - key: kubernetes.io/os
             operator: In
             values:
               - linux

DaemonSet added for Windows

px-csi-node-win-shared
px-installer-node-win

Windows Storage Class added
Airgap support added for windows pods

Testing :
Tested on OCP cluster with mix of linux and windows node
Tested on airgap cluster

pp511 · 2023-07-31T21:02:51Z

@nikita-bhatia nodeSelectorTerms need to be updated for stork and stork-scheduler deployments as well. Any particular reason that is not being handled in this PR? Check the following ticket for more context https://portworx.atlassian.net/browse/PWX-28210

github-actions · 2023-08-07T01:51:36Z

This PR is stale because it has been in review for 3 days with no activity.

deploy/crds/core_v1_storagecluster_crd.yaml

drivers/storage/portworx/manifest/manifest.go

drivers/storage/portworx/portworx.go

drivers/storage/portworx/manifest/manifest.go

deploy/windows/win.yaml

piyush-nimbalkar

I don't see the change to add the affinity to the storage cluster spec. Are we expecting the customer to add this or the spec generator?

drivers/storage/portworx/component/windows.go

drivers/storage/portworx/portworx.go

drivers/storage/portworx/testspec/px-csi-win-storageclass.yaml

drivers/storage/portworx/testspec/win.yaml

drivers/storage/portworx/component/windows.go

pkg/controller/storagecluster/stork.go

pp511

@nikita-bhatia I think we need the Windows feature behind a feature flag. Can you help me understand how is it being achieved?

nikita-bhatia · 2023-08-12T17:08:56Z

@nikita-bhatia I think we need the Windows feature behind a feature flag. Can you help me understand how is it being achieved?

@pp511 It was decided not to have feature flag. Currently, it is being achieved by checking the existence of windows node, if there is no windows node in cluster, this feature will not work.
CC : @pnookala-px

harsh-px · 2023-08-14T16:48:40Z

re: feature flag, I checked the code and I think we are fine without an explicit one. The windows component does not get enabled if no windows node is detected. So on all non-windows setups, the windows component code will be inactive.

@pp511 you okay with this?

harsh-px · 2023-08-14T16:49:00Z

@nikita-bhatia can we make travis green?

piyush-nimbalkar

Minor comments, but rest looks good.

Please update the branch and fix the failing csi unit test.

deploy/windows/px-installer-node-win.yaml

drivers/storage/portworx/deployment_test.go

drivers/storage/portworx/manifest/manifest.go

drivers/storage/portworx/portworx_test.go

drivers/storage/portworx/component/windows.go

pp511 · 2023-08-14T18:01:29Z

re: feature flag, I checked the code and I think we are fine without an explicit one. The windows component does not get enabled if no windows node is detected. So on all non-windows setups, the windows component code will be inactive.

@pp511 you okay with this?

@harsh-px I am good.

nikita-bhatia · 2023-08-14T18:51:07Z

@piyush-nimbalkar @pp511 re-requesting review, after rebasing branch to master and addressing all review comments.

codecov · 2023-08-14T19:10:09Z

Codecov Report

Patch coverage: 83.53% and project coverage change: +0.06% 🎉

Comparison is base (7843b18) 75.70% compared to head (8ce0671) 75.77%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1176      +/-   ##
==========================================
+ Coverage   75.70%   75.77%   +0.06%     
==========================================
  Files          64       65       +1     
  Lines       18246    18410     +164     
==========================================
+ Hits        13814    13951     +137     
- Misses       3449     3464      +15     
- Partials      983      995      +12

Files Changed	Coverage Δ
drivers/storage/portworx/component/windows.go	`80.85% <80.85%> (ø)`
drivers/storage/portworx/deployment.go	`95.91% <100.00%> (+0.01%)`	⬆️
drivers/storage/portworx/manifest/manifest.go	`91.50% <100.00%> (+0.11%)`	⬆️
drivers/storage/portworx/portworx.go	`83.10% <100.00%> (+0.03%)`	⬆️
drivers/storage/portworx/util/csi_generator.go	`100.00% <100.00%> (ø)`
drivers/storage/portworx/util/util.go	`76.18% <100.00%> (+0.32%)`	⬆️
pkg/migration/generate.go	`88.87% <100.00%> (+0.05%)`	⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

piyush-nimbalkar

Thanks for fixing all the comments @nikita-bhatia.
Everything looks good!

piyush-nimbalkar · 2023-08-15T04:08:52Z

deploy/windows/px-csi-node-win-shared.yaml

+apiVersion: apps/v1
+kind: DaemonSet
+metadata:
+  name: px-csi-node-win-shared


Why name the daemonset as shared? Let's rename this back to px-csi-node-win.
The previous comment was for the storageclass because it was a shared storageclass.

pp511

Lgtm. Thanks @nikita-bhatia

nikita-bhatia · 2023-08-15T06:18:47Z

Thanks @piyush-nimbalkar @pp511 for the approval, merging this to master.

* Add windows operator support * update daemonset labels * Replace nodeselecter with nodeaffinity * Add tests * add nodeselector for stork and stork-scheduler * remove nodeaffinity from components * add nodeselector as linux in storagecluster * address review comments * address review comments * address review comments * update windows specific affinity and tolerations * add px-installer daemonset for windows * fix failing tests * fix failing tests * remove fmt logs * set default csi-installer image for windows * fix failing test * fix review comments * fix failing test * rename px driver and node-win daemonset file names * remove fmt logs * rename px-csi-node-win-shared daemonset to px-csi-node-win

nikita-bhatia added 3 commits July 28, 2023 12:38

Add windows operator support

0cfb032

update daemonset labels

e334a79

Replace nodeselecter with nodeaffinity

b1fdf48

nikita-bhatia changed the title ~~Windows op support~~ Operator's Windows support Jul 31, 2023

github-actions bot added area/components area/manifest area/portworx area/specs size/m labels Jul 31, 2023

nikita-bhatia requested review from piyush-nimbalkar and ggriffiths August 1, 2023 03:39

github-actions bot added the stage/in-review label Aug 1, 2023

nikita-bhatia requested a review from a team August 1, 2023 03:39

Add tests

a075765

github-actions bot added size/l and removed size/m labels Aug 1, 2023

nikita-bhatia requested a review from zoxpx August 1, 2023 19:13

add nodeselector for stork and stork-scheduler

d3dc406

github-actions bot added the area/controller label Aug 1, 2023

nikita-bhatia changed the title ~~Operator's Windows support~~ PWX-31562 : Operator's Windows support Aug 3, 2023

github-actions bot added the stage/in-review-stale label Aug 7, 2023

piyush-nimbalkar reviewed Aug 9, 2023

View reviewed changes

remove nodeaffinity from components

97444e7

piyush-nimbalkar reviewed Aug 9, 2023

View reviewed changes

pp511 mentioned this pull request Aug 10, 2023

PWX-28210: Disable filter and prioritize for Windows Pods libopenstorage/stork#1470

Merged

nikita-bhatia added 3 commits August 10, 2023 16:36

add nodeselector as linux in storagecluster

ca43894

address review comments

4babd33

address review comments

c37fae7

set default csi-installer image for windows

65e406b

nikita-bhatia requested a review from piyush-nimbalkar August 11, 2023 10:55

pp511 requested changes Aug 11, 2023

View reviewed changes

github-actions bot removed the stage/in-review-stale label Aug 13, 2023

piyush-nimbalkar reviewed Aug 14, 2023

View reviewed changes

nikita-bhatia added 2 commits August 14, 2023 23:19

fix failing test

bece548

Merge remote-tracking branch 'origin/master' into windows_op_support

5d77680

fix review comments

0fd280e

nikita-bhatia requested a review from pp511 August 14, 2023 18:04

nikita-bhatia added 2 commits August 14, 2023 23:46

fix failing test

5373e0c

rename px driver and node-win daemonset file names

070ccb8

remove fmt logs

866c9b6

nikita-bhatia requested review from piyush-nimbalkar August 15, 2023 02:33

piyush-nimbalkar approved these changes Aug 15, 2023

View reviewed changes

github-actions bot added the stage/approved label Aug 15, 2023

pp511 approved these changes Aug 15, 2023

View reviewed changes

nikita-bhatia added 2 commits August 15, 2023 10:57

rename px-csi-node-win-shared daemonset to px-csi-node-win

dcf751a

Merge remote-tracking branch 'origin/master' into windows_op_support

8ce0671

nikita-bhatia merged commit 4810a06 into master Aug 15, 2023
8 checks passed

nikita-bhatia deleted the windows_op_support branch August 15, 2023 06:19

nikita-bhatia mentioned this pull request Aug 16, 2023

Cherrypick PWX-31562 : Operator's Windows support #1211

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PWX-31562 : Operator's Windows support #1176

PWX-31562 : Operator's Windows support #1176

nikita-bhatia commented Jul 31, 2023 •

edited

Loading

pp511 commented Jul 31, 2023 •

edited

Loading

github-actions bot commented Aug 7, 2023

piyush-nimbalkar left a comment

pp511 left a comment

nikita-bhatia commented Aug 12, 2023

harsh-px commented Aug 14, 2023

harsh-px commented Aug 14, 2023

piyush-nimbalkar left a comment

pp511 commented Aug 14, 2023

nikita-bhatia commented Aug 14, 2023 •

edited

Loading

codecov bot commented Aug 14, 2023 •

edited

Loading

piyush-nimbalkar left a comment

piyush-nimbalkar Aug 15, 2023

pp511 left a comment

nikita-bhatia commented Aug 15, 2023

PWX-31562 : Operator's Windows support #1176

PWX-31562 : Operator's Windows support #1176

Conversation

nikita-bhatia commented Jul 31, 2023 • edited Loading

pp511 commented Jul 31, 2023 • edited Loading

github-actions bot commented Aug 7, 2023

piyush-nimbalkar left a comment

Choose a reason for hiding this comment

pp511 left a comment

Choose a reason for hiding this comment

nikita-bhatia commented Aug 12, 2023

harsh-px commented Aug 14, 2023

harsh-px commented Aug 14, 2023

piyush-nimbalkar left a comment

Choose a reason for hiding this comment

pp511 commented Aug 14, 2023

nikita-bhatia commented Aug 14, 2023 • edited Loading

codecov bot commented Aug 14, 2023 • edited Loading

Codecov Report

piyush-nimbalkar left a comment

Choose a reason for hiding this comment

piyush-nimbalkar Aug 15, 2023

Choose a reason for hiding this comment

pp511 left a comment

Choose a reason for hiding this comment

nikita-bhatia commented Aug 15, 2023

nikita-bhatia commented Jul 31, 2023 •

edited

Loading

pp511 commented Jul 31, 2023 •

edited

Loading

nikita-bhatia commented Aug 14, 2023 •

edited

Loading

codecov bot commented Aug 14, 2023 •

edited

Loading