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

Issue 8754: add third party annotation support #8770

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
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
1 change: 1 addition & 0 deletions changelogs/unreleased/8770-Lyndon-Li
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix issue #8754, add third party annotation support for data mover
28 changes: 20 additions & 8 deletions pkg/controller/data_download_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -748,15 +748,27 @@
}
}

hostingPodAnnotation := map[string]string{}
for _, k := range util.ThirdPartyAnnotations {
if v, err := nodeagent.GetAnnotationValue(context.Background(), r.kubeClient, dd.Namespace, k, nodeOS); err != nil {
if err != nodeagent.ErrNodeAgentAnnotationNotFound {
log.WithError(err).Warnf("Failed to check node-agent annotation, skip adding host pod annotation %s", k)
}
} else {
hostingPodAnnotation[k] = v
}

Check warning on line 759 in pkg/controller/data_download_controller.go

View check run for this annotation

Codecov / codecov/patch

pkg/controller/data_download_controller.go#L755-L759

Added lines #L755 - L759 were not covered by tests
}

return exposer.GenericRestoreExposeParam{
TargetPVCName: dd.Spec.TargetVolume.PVC,
TargetNamespace: dd.Spec.TargetVolume.Namespace,
HostingPodLabels: hostingPodLabels,
Resources: r.podResources,
OperationTimeout: dd.Spec.OperationTimeout.Duration,
ExposeTimeout: r.preparingTimeout,
NodeOS: nodeOS,
RestorePVCConfig: r.restorePVCConfig,
TargetPVCName: dd.Spec.TargetVolume.PVC,
TargetNamespace: dd.Spec.TargetVolume.Namespace,
HostingPodLabels: hostingPodLabels,
HostingPodAnnotations: hostingPodAnnotation,
Resources: r.podResources,
OperationTimeout: dd.Spec.OperationTimeout.Duration,
ExposeTimeout: r.preparingTimeout,
NodeOS: nodeOS,
RestorePVCConfig: r.restorePVCConfig,
}, nil
}

Expand Down
36 changes: 24 additions & 12 deletions pkg/controller/data_upload_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -835,19 +835,31 @@
}
}

hostingPodAnnotation := map[string]string{}
for _, k := range util.ThirdPartyAnnotations {
if v, err := nodeagent.GetAnnotationValue(context.Background(), r.kubeClient, du.Namespace, k, nodeOS); err != nil {
if err != nodeagent.ErrNodeAgentAnnotationNotFound {
log.WithError(err).Warnf("Failed to check node-agent annotation, skip adding host pod annotation %s", k)
}
} else {
hostingPodAnnotation[k] = v
}

Check warning on line 846 in pkg/controller/data_upload_controller.go

View check run for this annotation

Codecov / codecov/patch

pkg/controller/data_upload_controller.go#L842-L846

Added lines #L842 - L846 were not covered by tests
}

return &exposer.CSISnapshotExposeParam{
SnapshotName: du.Spec.CSISnapshot.VolumeSnapshot,
SourceNamespace: du.Spec.SourceNamespace,
StorageClass: du.Spec.CSISnapshot.StorageClass,
HostingPodLabels: hostingPodLabels,
AccessMode: accessMode,
OperationTimeout: du.Spec.OperationTimeout.Duration,
ExposeTimeout: r.preparingTimeout,
VolumeSize: pvc.Spec.Resources.Requests[corev1.ResourceStorage],
Affinity: r.loadAffinity,
BackupPVCConfig: r.backupPVCConfig,
Resources: r.podResources,
NodeOS: nodeOS,
SnapshotName: du.Spec.CSISnapshot.VolumeSnapshot,
SourceNamespace: du.Spec.SourceNamespace,
StorageClass: du.Spec.CSISnapshot.StorageClass,
HostingPodLabels: hostingPodLabels,
HostingPodAnnotations: hostingPodAnnotation,
AccessMode: accessMode,
OperationTimeout: du.Spec.OperationTimeout.Duration,
ExposeTimeout: r.preparingTimeout,
VolumeSize: pvc.Spec.Resources.Requests[corev1.ResourceStorage],
Affinity: r.loadAffinity,
BackupPVCConfig: r.backupPVCConfig,
Resources: r.podResources,
NodeOS: nodeOS,
}, nil
}

Expand Down
8 changes: 7 additions & 1 deletion pkg/exposer/csi_snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ type CSISnapshotExposeParam struct {
// HostingPodLabels is the labels that are going to apply to the hosting pod
HostingPodLabels map[string]string

// HostingPodAnnotations is the annotations that are going to apply to the hosting pod
HostingPodAnnotations map[string]string

// OperationTimeout specifies the time wait for resources operations in Expose
OperationTimeout time.Duration

Expand Down Expand Up @@ -211,6 +214,7 @@ func (e *csiSnapshotExposer) Expose(ctx context.Context, ownerObject corev1.Obje
backupPVC,
csiExposeParam.OperationTimeout,
csiExposeParam.HostingPodLabels,
csiExposeParam.HostingPodAnnotations,
csiExposeParam.Affinity,
csiExposeParam.Resources,
backupPVCReadOnly,
Expand Down Expand Up @@ -523,6 +527,7 @@ func (e *csiSnapshotExposer) createBackupPod(
backupPVC *corev1.PersistentVolumeClaim,
operationTimeout time.Duration,
label map[string]string,
annotation map[string]string,
affinity *kube.LoadAffinity,
resources corev1.ResourceRequirements,
backupPVCReadOnly bool,
Expand Down Expand Up @@ -633,7 +638,8 @@ func (e *csiSnapshotExposer) createBackupPod(
Controller: boolptr.True(),
},
},
Labels: label,
Labels: label,
Annotations: annotation,
},
Spec: corev1.PodSpec{
TopologySpreadConstraints: []corev1.TopologySpreadConstraint{
Expand Down
10 changes: 7 additions & 3 deletions pkg/exposer/generic_restore.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ type GenericRestoreExposeParam struct {
// HostingPodLabels is the labels that are going to apply to the hosting pod
HostingPodLabels map[string]string

// HostingPodAnnotations is the annotations that are going to apply to the hosting pod
HostingPodAnnotations map[string]string

// Resources defines the resource requirements of the hosting pod
Resources corev1.ResourceRequirements

Expand Down Expand Up @@ -119,7 +122,7 @@ func (e *genericRestoreExposer) Expose(ctx context.Context, ownerObject corev1.O
return errors.Errorf("Target PVC %s/%s has already been bound, abort", param.TargetNamespace, param.TargetPVCName)
}

restorePod, err := e.createRestorePod(ctx, ownerObject, targetPVC, param.OperationTimeout, param.HostingPodLabels, selectedNode, param.Resources, param.NodeOS)
restorePod, err := e.createRestorePod(ctx, ownerObject, targetPVC, param.OperationTimeout, param.HostingPodLabels, param.HostingPodAnnotations, selectedNode, param.Resources, param.NodeOS)
if err != nil {
return errors.Wrapf(err, "error to create restore pod")
}
Expand Down Expand Up @@ -374,7 +377,7 @@ func (e *genericRestoreExposer) RebindVolume(ctx context.Context, ownerObject co
}

func (e *genericRestoreExposer) createRestorePod(ctx context.Context, ownerObject corev1.ObjectReference, targetPVC *corev1.PersistentVolumeClaim,
operationTimeout time.Duration, label map[string]string, selectedNode string, resources corev1.ResourceRequirements, nodeType string) (*corev1.Pod, error) {
operationTimeout time.Duration, label map[string]string, annotation map[string]string, selectedNode string, resources corev1.ResourceRequirements, nodeType string) (*corev1.Pod, error) {
restorePodName := ownerObject.Name
restorePVCName := ownerObject.Name

Expand Down Expand Up @@ -464,7 +467,8 @@ func (e *genericRestoreExposer) createRestorePod(ctx context.Context, ownerObjec
Controller: boolptr.True(),
},
},
Labels: label,
Labels: label,
Annotations: annotation,
},
Spec: corev1.PodSpec{
TopologySpreadConstraints: []corev1.TopologySpreadConstraint{
Expand Down
28 changes: 26 additions & 2 deletions pkg/nodeagent/node_agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,9 @@
)

var (
ErrDaemonSetNotFound = errors.New("daemonset not found")
ErrNodeAgentLabelNotFound = errors.New("node-agent label not found")
ErrDaemonSetNotFound = errors.New("daemonset not found")
ErrNodeAgentLabelNotFound = errors.New("node-agent label not found")
ErrNodeAgentAnnotationNotFound = errors.New("node-agent annotation not found")
)

type LoadConcurrency struct {
Expand Down Expand Up @@ -225,3 +226,26 @@

return val, nil
}

func GetAnnotationValue(ctx context.Context, kubeClient kubernetes.Interface, namespace string, key string, osType string) (string, error) {
dsName := daemonSet
if osType == kube.NodeOSWindows {
dsName = daemonsetWindows
}

Check warning on line 234 in pkg/nodeagent/node_agent.go

View check run for this annotation

Codecov / codecov/patch

pkg/nodeagent/node_agent.go#L233-L234

Added lines #L233 - L234 were not covered by tests

ds, err := kubeClient.AppsV1().DaemonSets(namespace).Get(ctx, dsName, metav1.GetOptions{})
if err != nil {
return "", errors.Wrapf(err, "error getting %s daemonset", dsName)
}

if ds.Spec.Template.Annotations == nil {
return "", ErrNodeAgentAnnotationNotFound
}

val, found := ds.Spec.Template.Annotations[key]
if !found {
return "", ErrNodeAgentAnnotationNotFound
}

return val, nil
}
129 changes: 129 additions & 0 deletions pkg/nodeagent/node_agent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -461,3 +461,132 @@ func TestGetLabelValue(t *testing.T) {
})
}
}

func TestGetAnnotationValue(t *testing.T) {
daemonSet := &appsv1.DaemonSet{
ObjectMeta: metav1.ObjectMeta{
Namespace: "fake-ns",
Name: "node-agent",
},
TypeMeta: metav1.TypeMeta{
Kind: "DaemonSet",
},
}

daemonSetWithOtherAnnotation := &appsv1.DaemonSet{
ObjectMeta: metav1.ObjectMeta{
Namespace: "fake-ns",
Name: "node-agent",
},
TypeMeta: metav1.TypeMeta{
Kind: "DaemonSet",
},
Spec: appsv1.DaemonSetSpec{
Template: corev1.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
"fake-other-annotation": "fake-value-1",
},
},
},
},
}

daemonSetWithAnnotation := &appsv1.DaemonSet{
ObjectMeta: metav1.ObjectMeta{
Namespace: "fake-ns",
Name: "node-agent",
},
TypeMeta: metav1.TypeMeta{
Kind: "DaemonSet",
},
Spec: appsv1.DaemonSetSpec{
Template: corev1.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
"fake-annotation": "fake-value-2",
},
},
},
},
}

daemonSetWithEmptyAnnotation := &appsv1.DaemonSet{
ObjectMeta: metav1.ObjectMeta{
Namespace: "fake-ns",
Name: "node-agent",
},
TypeMeta: metav1.TypeMeta{
Kind: "DaemonSet",
},
Spec: appsv1.DaemonSetSpec{
Template: corev1.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
"fake-annotation": "",
},
},
},
},
}

tests := []struct {
name string
kubeClientObj []runtime.Object
namespace string
expectedValue string
expectErr string
}{
{
name: "ds get error",
namespace: "fake-ns",
expectErr: "error getting node-agent daemonset: daemonsets.apps \"node-agent\" not found",
},
{
name: "no annotation",
namespace: "fake-ns",
kubeClientObj: []runtime.Object{
daemonSet,
},
expectErr: ErrNodeAgentAnnotationNotFound.Error(),
},
{
name: "no expecting annotation",
namespace: "fake-ns",
kubeClientObj: []runtime.Object{
daemonSetWithOtherAnnotation,
},
expectErr: ErrNodeAgentAnnotationNotFound.Error(),
},
{
name: "expecting annotation",
namespace: "fake-ns",
kubeClientObj: []runtime.Object{
daemonSetWithAnnotation,
},
expectedValue: "fake-value-2",
},
{
name: "expecting empty annotation",
namespace: "fake-ns",
kubeClientObj: []runtime.Object{
daemonSetWithEmptyAnnotation,
},
expectedValue: "",
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
fakeKubeClient := fake.NewSimpleClientset(test.kubeClientObj...)

value, err := GetAnnotationValue(context.TODO(), fakeKubeClient, test.namespace, "fake-annotation", kube.NodeOSLinux)
if test.expectErr == "" {
assert.NoError(t, err)
assert.Equal(t, test.expectedValue, value)
} else {
assert.EqualError(t, err, test.expectErr)
}
})
}
}
4 changes: 4 additions & 0 deletions pkg/util/third_party.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,7 @@ package util
var ThirdPartyLabels = []string{
"azure.workload.identity/use",
}

var ThirdPartyAnnotations = []string{
"iam.amazonaws.com/role",
}
Loading