Fix OADP-4623: OpenShift on IBMCloud setup for OADP #1482
Conversation
|
@shubham-pampattiwar: This pull request references OADP-4623 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.17.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci-robot
added
the
jira/valid-reference
|
/cherry-pick oadp-1.4 |
|
@shubham-pampattiwar: once the present PR merges, I will cherry-pick it on top of oadp-1.4 in a new PR and assign it to you. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
openshift-ci
bot
added
the
approved
|
Is a valid test scenario to run block e2e test on IBM (it never worked for me)? Ref https://github.com/openshift/oadp-operator/blob/master/tests/e2e/backup_restore_suite_test.go#L396 |
|
@mateusoliveira43 yes this should help with the block e2e on IBM Cloud. Testing block volume app on IBM Cloud would certainly help here. |
There was a problem hiding this comment.
Please make updates as https://github.com/vmware-tanzu/velero/pull/8077/files#diff-a0121c33bae4bfd2067f10178ed5105b59a43ffe490df868bac0a78054551920 modifies prior doc this PR originally was based on.
|
/hold PR needs an update, refer: vmware-tanzu/velero#8077 |
openshift-ci
bot
added
the
do-not-merge/hold
|
@shubham-pampattiwar: This pull request references OADP-4623 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.17.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
PR updated for host-plugins path changes for node-agent. PTAL ! |
| Cluster = "cluster" | ||
| IBMCloudPlatform = "IBMCloud" | ||
| GenericPVHostPath = "/var/lib/kubelet/pods" | ||
| IBMCloudPVHostPath = "/var/data/kubelet/pods" |
There was a problem hiding this comment.
I think this should be deleted
what IBM changes is just host-plugins path, no?
There was a problem hiding this comment.
I think yesterday in scrum we discussed that we need both.
cc: @sseago
There was a problem hiding this comment.
I recall both are needed to allow a symlink to work. Pod and Host iiuc
There was a problem hiding this comment.
what I remember was that we need to mount both /var/lib/kubelet and /var/data/kubelet on pod
I tested only changing host-plugins path and e2e block datamover worked for me in IBM (which never worked before)
If we need both, then velero docs are still wrong https://velero.io/docs/v1.14/csi-snapshot-data-movement/#configure-node-agent-daemonset-spec
There was a problem hiding this comment.
yeah we might need to update the docs upstream, I will do that once this PR is completed.
|
@weshayutin I will build and try the changes. |
|
/retest |
thank you sir! |
Changes look good. node-agent config is as expected on IBM Cloud. Status is healthy. And node-agent pods have access to the volumes directly. DataUpload successful For as little as it matters, you have my approval for this change. |
shubham-pampattiwar
dismissed stale reviews from kaovilai and weshayutin
via
98a67e6
shubham-pampattiwar
requested review from
mateusoliveira43,
weshayutin and
kaovilai
| @@ -2898,6 +3032,13 @@ func TestDPAReconciler_buildNodeAgentDaemonset(t *testing.T) { | |||
| }, | |||
| }, | |||
| wantErr: false, | |||
| clientObjects: []client.Object{ | |||
There was a problem hiding this comment.
are we adding this to all tests?
if yes, I would append it to test clientObjects inside t.Run
There was a problem hiding this comment.
I think this is fine. Having a clientObject arg for each test case makes this future proof. We might need/write some specific test cases in the future that might need object mocking on per test case basis. And having clientObject array helps in that scenario.
|
/retest |
|
/retest |
|
@shubham-pampattiwar: all tests passed! Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
Testcase details: Completed Backup: Completed DataUpload: Completed Restore: Completed DataDownload: Data in app is also restored: |
openshift-ci
bot
removed
the
do-not-merge/hold
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kaovilai, mateusoliveira43, shubham-pampattiwar, weshayutin The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@shubham-pampattiwar: #1482 failed to apply on top of branch "oadp-1.4": In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
* Fix OADP-4623: OpenShift on IBMCLoud setup for OADP * fix unit tests and minor updates * add updates for host-plugins host path * fix unit tests * lint fix (cherry picked from commit b8fb9dd)
Why the changes were made
The default hostpath /var/lib/kubelet/pods cannot find PersistentVolumeClaims with volumeMode: Block on host.
The correct hostpath for OpenShift on IBM Cloud is /var/data/kubelet/pods
Similarly for host-plugins the correct hostpath is /var/data/kubelet/plugins
We are adding the ability to check for OpenShift infrastructure and update the hostpath and hostplugins for node-agent daemonset in case of OpenShift on IBMCloud
Note: The hostpath set via operator CSV env vars will take precedence (
RESTIC_PV_HOSTPATHandFS_PV_HOSTPATH)Similarly for host-plugins CSV env var will take precedence (
PLUGINS_HOSTPATH)How to test the changes made
/var/data/kubelet/pods/var/lib/kubelet/podsFollow the same steps as above for host-plugins path:
var/data/kubelet/pluginsvar/lib/kubelet/plugins