PWX-36475 : Creating node PDB for parallel upgrades #1508
Conversation
| shouldRun, shouldContinueRunning, err := k8sutil.CheckPredicatesForStoragePod(&node, cluster, nil) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| if shouldRun || shouldContinueRunning { |
There was a problem hiding this comment.
Why do we need to check if that k8s node can run a portworx pod or not? Don't we need to only check if the portworx node is part of the current cluster or not? Basically this can be a simple map -
for _, node := range k8sNodeList.Items {
k8sNodes[node.Name] = true
}
There was a problem hiding this comment.
It was mentioned that we will create PDB only for k8s nodes that can run px right? So during surge upgrade when a node is being removed from the cluster, it shouldn't run px and we wont create a node PDB for it .
There was a problem hiding this comment.
For now I will make the change suggested, but lets discuss why we need to create node PDB for k8s nodes that should not run px pod
There was a problem hiding this comment.
If a portworx node has scheduler name which is part of the cluster, then we should protect that k8s node from upgrading irrespective of whether currently the k8s node should run portworx or not, right?
Let's discuss more on this offline to make sure we are covering all cases correctly.
There was a problem hiding this comment.
I assumed a kubernetes node when it goes to the state of should not run portworx, then px will be deleted from that node soon anyways, so why should we create pdb for it only to delete it in sometime?
| err = driver.PreInstall(cluster) | ||
| require.NoError(t, err) | ||
|
|
||
| // PDB is created for 2 storage nodes and kvdb |
There was a problem hiding this comment.
We are not setting the spec.kvdb.internal field in the storage cluster. Then why is the kvdb pdb getting created here?
There was a problem hiding this comment.
It is creating kvdb pdb as kvdb spec itself is nil, so it doesnt check if we set internal or not
There was a problem hiding this comment.
if cluster.Spec.Kvdb != nil && !cluster.Spec.Kvdb.Internal the first condition fails and thus creates kvdb pdb. In the other tests, kvdb is being set / total PDB counts is not used
svijaykumar-px
force-pushed
the
PWX-36475
branch
from
45c0ae7 to
9ce2a4a
Compare
|
This PR is stale because it has been in review for 3 days with no activity. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1508 +/- ##
==========================================
- Coverage 75.97% 75.97% -0.01%
==========================================
Files 77 77
Lines 20483 20578 +95
==========================================
+ Hits 15562 15634 +72
- Misses 3837 3850 +13
- Partials 1084 1094 +10 ☔ View full report in Codecov by Sentry. |
|
This PR is stale because it has been in review for 3 days with no activity. |
Conflicts: drivers/storage/portworx/component/disruption_budget.go
| errors := []error{} | ||
| for _, node := range nodesNeedingPDB { | ||
| minAvailable := intstr.FromInt(1) | ||
| PDBName := "px-" + node |
There was a problem hiding this comment.
nit: internal variables should have lower case.
*pdbName
What this PR does / why we need it: Creating Node PDB for every node in the cluster which is a part of quorum. This will be used in parallel upgrades. The PDB for every node is created with the name px-storage- and matches with the node name label of every portworx pod
Which issue(s) this PR fixes (optional)
Closes # PWX-36475
Special notes for your reviewer: This per node PDB will only be created when portworx version is greater than or equal to 3.1.2