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

Fix OADP-4623: OpenShift on IBMCloud setup for OADP #1482

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions bundle/manifests/oadp-operator.clusterserviceversion.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -648,6 +648,8 @@ spec:
- infrastructures
verbs:
- get
- list
- watch
kaovilai marked this conversation as resolved.
Show resolved Hide resolved
- apiGroups:
- cloudcredential.openshift.io
resources:
Expand Down Expand Up @@ -904,6 +906,9 @@ spec:
valueFrom:
fieldRef:
fieldPath: metadata.annotations['olm.targetNamespaces']
- name: RESTIC_PV_HOSTPATH
- name: FS_PV_HOSTPATH
- name: PLUGINS_HOSTPATH
- name: RELATED_IMAGE_VELERO
value: quay.io/konveyor/velero:latest
- name: RELATED_IMAGE_VELERO_RESTORE_HELPER
Expand Down
6 changes: 6 additions & 0 deletions config/manager/manager.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,12 @@ spec:
valueFrom:
fieldRef:
fieldPath: metadata.namespace
- name: RESTIC_PV_HOSTPATH
shubham-pampattiwar marked this conversation as resolved.
Show resolved Hide resolved
value: ""
- name: FS_PV_HOSTPATH
value: ""
- name: PLUGINS_HOSTPATH
value: ""
- name: RELATED_IMAGE_VELERO
value: quay.io/konveyor/velero:latest
- name: RELATED_IMAGE_VELERO_RESTORE_HELPER
Expand Down
2 changes: 2 additions & 0 deletions config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ rules:
- infrastructures
verbs:
- get
- list
- watch
- apiGroups:
- cloudcredential.openshift.io
resources:
Expand Down
6 changes: 6 additions & 0 deletions controllers/bsl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

"github.com/go-logr/logr"
"github.com/google/go-cmp/cmp"
configv1 "github.com/openshift/api/config/v1"
velerov1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -36,6 +37,11 @@ func getSchemeForFakeClient() (*runtime.Scheme, error) {
return nil, err
}

err = configv1.AddToScheme((scheme.Scheme))
if err != nil {
return nil, err
}

return scheme.Scheme, nil
}

Expand Down
93 changes: 80 additions & 13 deletions controllers/nodeagent.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"reflect"

"github.com/go-logr/logr"
configv1 "github.com/openshift/api/config/v1"
"github.com/operator-framework/operator-lib/proxy"
"github.com/vmware-tanzu/velero/pkg/install"
appsv1 "k8s.io/api/apps/v1"
Expand All @@ -25,14 +26,24 @@ import (
)

const (
ResticRestoreHelperCM = "restic-restore-action-config"
FsRestoreHelperCM = "fs-restore-action-config"
HostPods = "host-pods"
HostPlugins = "host-plugins"
ResticRestoreHelperCM = "restic-restore-action-config"
FsRestoreHelperCM = "fs-restore-action-config"
HostPods = "host-pods"
HostPlugins = "host-plugins"
Cluster = "cluster"
IBMCloudPlatform = "IBMCloud"
GenericPVHostPath = "/var/lib/kubelet/pods"
IBMCloudPVHostPath = "/var/data/kubelet/pods"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be deleted

what IBM changes is just host-plugins path, no?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think yesterday in scrum we discussed that we need both.
cc: @sseago

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recall both are needed to allow a symlink to work. Pod and Host iiuc

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

@shubham-pampattiwar shubham-pampattiwar Aug 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah we might need to update the docs upstream, I will do that once this PR is completed.

GenericPluginsHostPath = "/var/lib/kubelet/plugins"
IBMCloudPluginsHostPath = "/var/data/kubelet/plugins"
ResticPVHostPathEnvVar = "RESTIC_PV_HOSTPATH"
FSPVHostPathEnvVar = "FS_PV_HOSTPATH"
PluginsHostPathEnvVar = "PLUGINS_HOSTPATH"
)

var (
fsPvHostPath string = getFsPvHostPath()
fsPvHostPath = getFsPvHostPath("")
pluginsHostPath = getPluginsHostPath("")

// v1.MountPropagationHostToContainer is a const. Const cannot be pointed to.
// we need to declare mountPropagationToHostContainer so that we have an address to point to
Expand All @@ -47,19 +58,40 @@ var (
}
)

func getFsPvHostPath() string {
env := os.Getenv("RESTIC_PV_HOSTPATH")
envFs := os.Getenv("FS_PV_HOSTPATH")
// getFsPvHostPath returns the host path for persistent volumes based on the platform type.
func getFsPvHostPath(platformType string) string {
// Check if environment variables are set for host paths
if envFs := os.Getenv(FSPVHostPathEnvVar); envFs != "" {
return envFs
}

if env != "" {
if env := os.Getenv(ResticPVHostPathEnvVar); env != "" {
return env
}

if envFs != "" {
return envFs
// Return platform-specific host paths
switch platformType {
case IBMCloudPlatform:
return IBMCloudPVHostPath
default:
return GenericPVHostPath
}
}

// getPluginsHostPath returns the host path for persistent volumes based on the platform type.
func getPluginsHostPath(platformType string) string {
// Check if environment var is set for host plugins
if env := os.Getenv(PluginsHostPathEnvVar); env != "" {
return env
}

return "/var/lib/kubelet/pods"
// Return platform-specific host paths
switch platformType {
case IBMCloudPlatform:
return IBMCloudPluginsHostPath
default:
return GenericPluginsHostPath
}
}

func getNodeAgentObjectMeta(r *DPAReconciler) metav1.ObjectMeta {
Expand Down Expand Up @@ -281,10 +313,22 @@ func (r *DPAReconciler) customizeNodeAgentDaemonset(dpa *oadpv1alpha1.DataProtec
},
})

// check platform type
platformType, err := r.getPlatformType()
if err != nil {
return nil, fmt.Errorf("error checking platform type: %s", err)
}
// update nodeAgent host PV path
for i, vol := range ds.Spec.Template.Spec.Volumes {
if vol.Name == HostPods {
ds.Spec.Template.Spec.Volumes[i].HostPath.Path = getFsPvHostPath()
ds.Spec.Template.Spec.Volumes[i].HostPath.Path = getFsPvHostPath(platformType)
}
}

// update nodeAgent plugins host path
for i, vol := range ds.Spec.Template.Spec.Volumes {
if vol.Name == HostPlugins {
ds.Spec.Template.Spec.Volumes[i].HostPath.Path = getPluginsHostPath(platformType)
}
}

Expand All @@ -310,6 +354,13 @@ func (r *DPAReconciler) customizeNodeAgentDaemonset(dpa *oadpv1alpha1.DataProtec
Name: "certs",
MountPath: "/etc/ssl/certs",
})

// update nodeAgent plugins volume mount host path
for v, volumeMount := range nodeAgentContainer.VolumeMounts {
if volumeMount.Name == HostPlugins {
nodeAgentContainer.VolumeMounts[v].MountPath = getPluginsHostPath(platformType)
}
}
// append PodConfig envs to nodeAgent container
if useResticConf {
if dpa.Spec.Configuration.Restic.PodConfig != nil && dpa.Spec.Configuration.Restic.PodConfig.Env != nil {
Expand Down Expand Up @@ -452,3 +503,19 @@ func (r *DPAReconciler) updateFsRestoreHelperCM(fsRestoreHelperCM *corev1.Config

return nil
}

// getPlatformType fetches the cluster infrastructure object and returns the platform type.
func (r *DPAReconciler) getPlatformType() (string, error) {
infra := &configv1.Infrastructure{}
key := types.NamespacedName{Name: Cluster}
if err := r.Get(r.Context, key, infra); err != nil {
return "", err
}

if platformStatus := infra.Status.PlatformStatus; platformStatus != nil {
if platformType := platformStatus.Type; platformType != "" {
return string(platformType), nil
}
}
return "", nil
}
Loading