fs-backup for clusters with windows nodes #8518
Conversation
Lyndon-Li
force-pushed
the
fail-fs-backup-on-windows-nodes
branch
from
54dbb7f to
96cd224
Compare
Lyndon-Li
force-pushed
the
fail-fs-backup-on-windows-nodes
branch
5 times, most recently
from
2e3e3f0 to
33efb69
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8518 +/- ##
=======================================
Coverage 59.16% 59.17%
=======================================
Files 369 370 +1
Lines 39413 39487 +74
=======================================
+ Hits 23319 23366 +47
- Misses 14624 14647 +23
- Partials 1470 1474 +4 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Lyndon-Li <[email protected]>
Lyndon-Li
force-pushed
the
fail-fs-backup-on-windows-nodes
branch
from
33efb69 to
a711b10
Compare
Lyndon-Li
changed the title
Signed-off-by: Lyndon-Li <[email protected]>
| } | ||
| } | ||
|
|
||
| if kube.WithWindowsNode(s.ctx, s.crClient, s.logger) { |
There was a problem hiding this comment.
Do we need to check this if the Windows node agent isn't enabled?
There was a problem hiding this comment.
We don't need to do this.
This is a sanity check and needs to state the fact in all cases when Windows/linux nodes exist but fs-backup/data mover won't work in the nodes.
Signed-off-by: Lyndon-Li <[email protected]>
| } | ||
| } | ||
|
|
||
| if !allNodeLabeled { |
There was a problem hiding this comment.
The variable allNodeLabeled is useless.
If the expected OS is found, the code returns directly inside the for loop.
We can log the warning message directly just before the last return false.
This is tiny, feel free to change it or not.
Approved.
There was a problem hiding this comment.
When return false is reached, there are two possibilities: 1. all nodes are with OS label, but are not with the expected OS type value; 2. not all nodes with OS label. So it's better to use allNodeLabeled to track these two cases.
Lyndon-Li
changed the title
Changed it to |
Make fs-backup work on linux nodes with the new Velero deployment and disable fs-backup if the source/target pod is running in non-linux node (#8424)