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

[OADP-452 , OADP-490] Add labels to dpa podConfig, Fix velero pod (match)labels missing deploy: velero #668

Merged
merged 19 commits into from
May 11, 2022
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
7 changes: 6 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -296,9 +296,9 @@ bundle-build: ## Build the bundle image.
bundle-push: ## Push the bundle image.
$(MAKE) docker-push IMG=$(BUNDLE_IMG)

GIT_REV:=$(shell git rev-parse --short HEAD)
## Build current branch operator image, bundle image, push and install via OLM
.PHONY: deploy-olm
deploy-olm: GIT_REV=$(shell git rev-parse --short HEAD)
deploy-olm: THIS_OPERATOR_IMAGE?=ttl.sh/oadp-operator-$(GIT_REV):1h # Set target specific variable
deploy-olm: THIS_BUNDLE_IMAGE?=ttl.sh/oadp-operator-bundle-$(GIT_REV):1h # Set target specific variable
deploy-olm: DEPLOY_TMP:=$(shell mktemp -d)/ # Set target specific variable
Expand Down Expand Up @@ -350,6 +350,11 @@ endif
catalog-build: opm ## Build a catalog image.
$(OPM) index add --container-tool docker --mode semver --tag $(CATALOG_IMG) --bundles $(BUNDLE_IMGS) $(FROM_INDEX_OPT)

# For testing oeprator upgrade
# opm upgrade
catalog-build-replaces: opm ## Build a catalog image using replace mode
$(OPM) index add --container-tool docker --mode replaces --tag $(CATALOG_IMG) --bundles $(BUNDLE_IMGS) $(FROM_INDEX_OPT)

# Push the catalog image.
.PHONY: catalog-push
catalog-push: ## Push a catalog image.
Expand Down
3 changes: 3 additions & 0 deletions api/v1alpha1/oadp_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,9 @@ type VeleroConfig struct {

// PodConfig defines the pod configuration options
type PodConfig struct {
// Labels to add to pods
// +optional
Labels map[string]string `json:"labels,omitempty"`
// NodeSelector defines the nodeSelector to be supplied to Restic podSpec
// +optional
NodeSelector map[string]string `json:"nodeSelector,omitempty"`
Expand Down
7 changes: 7 additions & 0 deletions api/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions bundle/manifests/oadp-operator.clusterserviceversion.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -628,14 +628,15 @@ spec:
- security.openshift.io
resourceNames:
- privileged
- velero-privileged
resources:
- securitycontextconstraints
verbs:
- use
serviceAccountName: velero
deployments:
- name: openshift-adp-controller-manager
- label:
control-plane: controller-manager
name: openshift-adp-controller-manager
spec:
replicas: 1
selector:
Expand Down
10 changes: 10 additions & 0 deletions bundle/manifests/oadp.openshift.io_dataprotectionapplications.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,11 @@ spec:
podConfig:
description: Pod specific configuration
properties:
labels:
additionalProperties:
type: string
description: Labels to add to pods
type: object
nodeSelector:
additionalProperties:
type: string
Expand Down Expand Up @@ -329,6 +334,11 @@ spec:
podConfig:
description: Pod specific configuration
properties:
labels:
additionalProperties:
type: string
description: Labels to add to pods
type: object
nodeSelector:
additionalProperties:
type: string
Expand Down
10 changes: 10 additions & 0 deletions config/crd/bases/oadp.openshift.io_dataprotectionapplications.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,11 @@ spec:
podConfig:
description: Pod specific configuration
properties:
labels:
additionalProperties:
type: string
description: Labels to add to pods
type: object
nodeSelector:
additionalProperties:
type: string
Expand Down Expand Up @@ -331,6 +336,11 @@ spec:
podConfig:
description: Pod specific configuration
properties:
labels:
additionalProperties:
type: string
description: Labels to add to pods
type: object
nodeSelector:
additionalProperties:
type: string
Expand Down
47 changes: 44 additions & 3 deletions controllers/restic.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,22 @@ func (r *DPAReconciler) ReconcileResticDaemonset(log logr.Logger) (bool, error)
// Deployment selector is immutable so we set this value only if
// a new object is going to be created
if ds.ObjectMeta.CreationTimestamp.IsZero() {
ds.Spec.Selector = resticLabelSelector
if ds.Spec.Selector == nil {
ds.Spec.Selector = &metav1.LabelSelector{}
}
var err error
if ds.Spec.Selector == nil {
ds.Spec.Selector = &metav1.LabelSelector{
MatchLabels: make(map[string]string),
}
}
if ds.Spec.Selector.MatchLabels == nil {
ds.Spec.Selector.MatchLabels = make(map[string]string)
}
ds.Spec.Selector.MatchLabels, err = common.AppendUniqueLabels(ds.Spec.Selector.MatchLabels, resticLabelSelector.MatchLabels)
if err != nil {
return fmt.Errorf("failed to append labels to selector: %s", err)
}
}

if err := controllerutil.SetControllerReference(&dpa, ds, r.Scheme); err != nil {
Expand All @@ -114,6 +129,19 @@ func (r *DPAReconciler) ReconcileResticDaemonset(log logr.Logger) (bool, error)
})

if err != nil {
if errors.IsInvalid(err) {
cause, isStatusCause := errors.StatusCause(err, metav1.CauseTypeFieldValueInvalid)
if isStatusCause && cause.Field == "spec.selector" {
// recreate deployment
// TODO: check for in-progress backup/restore to wait for it to finish
log.Info("Found immutable selector from previous daemonset, recreating restic daemonset")
err := r.Delete(r.Context, ds)
if err != nil {
return false, err
}
return r.ReconcileResticDaemonset(log)
}
}
return false, err
}

Expand Down Expand Up @@ -149,16 +177,29 @@ func (r *DPAReconciler) buildResticDaemonset(dpa *oadpv1alpha1.DataProtectionApp
install.WithAnnotations(dpa.Spec.PodAnnotations),
install.WithSecret(false))
// Update Items in ObjectMeta
dsName := ds.Name
ds.TypeMeta = installDs.TypeMeta
// Update Spec
ds.Spec = installDs.Spec
ds.Labels = installDs.Labels
ds.ObjectMeta = installDs.ObjectMeta
ds.Name = dsName

return r.customizeResticDaemonset(dpa, ds)
}

func (r *DPAReconciler) customizeResticDaemonset(dpa *oadpv1alpha1.DataProtectionApplication, ds *appsv1.DaemonSet) (*appsv1.DaemonSet, error) {

if dpa.Spec.Configuration.Restic == nil {
// if restic is not configured, therefore not enabled, return early.
return nil, nil
}
// add custom pod labels
if dpa.Spec.Configuration.Restic.PodConfig != nil && dpa.Spec.Configuration.Restic.PodConfig.Labels != nil {
var err error
ds.Spec.Template.Labels, err = common.AppendUniqueLabels(ds.Spec.Template.Labels, dpa.Spec.Configuration.Restic.PodConfig.Labels)
if err != nil {
return nil, fmt.Errorf("restic daemonset template custom label: %s", err)
}
}
// customize specs
ds.Spec.Selector = resticLabelSelector
ds.Spec.UpdateStrategy = appsv1.DaemonSetUpdateStrategy{
Expand Down
166 changes: 166 additions & 0 deletions controllers/restic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,172 @@ func TestDPAReconciler_buildResticDaemonset(t *testing.T) {
},
},
},
{
name: "podConfig label for velero and restic",
args: args{
&oadpv1alpha1.DataProtectionApplication{
Spec: oadpv1alpha1.DataProtectionApplicationSpec{
Configuration: &oadpv1alpha1.ApplicationConfig{
Restic: &oadpv1alpha1.ResticConfig{
PodConfig: &oadpv1alpha1.PodConfig{
Labels: map[string]string{
"resticLabel": "this is a label",
},
},
},
Velero: &oadpv1alpha1.VeleroConfig{
PodConfig: &oadpv1alpha1.PodConfig{
Labels: map[string]string{
"veleroLabel": "this is a label",
},
},
},
},
},
}, &appsv1.DaemonSet{
ObjectMeta: getResticObjectMeta(r),
},
},
wantErr: false,
want: &appsv1.DaemonSet{
ObjectMeta: getResticObjectMeta(r),
TypeMeta: metav1.TypeMeta{
Kind: "DaemonSet",
APIVersion: appsv1.SchemeGroupVersion.String(),
},
Spec: appsv1.DaemonSetSpec{
UpdateStrategy: appsv1.DaemonSetUpdateStrategy{
Type: appsv1.RollingUpdateDaemonSetStrategyType,
},
Selector: resticLabelSelector,
Template: v1.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{
"component": common.Velero,
"name": common.Restic,
"resticLabel": "this is a label",
},
},
Spec: v1.PodSpec{
NodeSelector: dpa.Spec.Configuration.Restic.PodConfig.NodeSelector,
ServiceAccountName: common.Velero,
SecurityContext: &v1.PodSecurityContext{
RunAsUser: pointer.Int64(0),
SupplementalGroups: dpa.Spec.Configuration.Restic.SupplementalGroups,
},
Volumes: []v1.Volume{
// Cloud Provider volumes are dynamically added in the for loop below
{
Name: HostPods,
VolumeSource: v1.VolumeSource{
HostPath: &v1.HostPathVolumeSource{
Path: resticPvHostPath,
},
},
},
{
Name: "scratch",
VolumeSource: v1.VolumeSource{
EmptyDir: &v1.EmptyDirVolumeSource{},
},
},
{
Name: "certs",
VolumeSource: v1.VolumeSource{
EmptyDir: &v1.EmptyDirVolumeSource{},
},
},
},
Tolerations: dpa.Spec.Configuration.Restic.PodConfig.Tolerations,
Containers: []v1.Container{
{
Name: common.Restic,
SecurityContext: &v1.SecurityContext{
Privileged: pointer.Bool(true),
},
Image: getVeleroImage(&dpa),
ImagePullPolicy: v1.PullAlways,
Resources: r.getResticResourceReqs(&dpa), //setting default.
Command: []string{
"/velero",
},
Args: []string{
"restic",
"server",
},
VolumeMounts: []v1.VolumeMount{
{
Name: "host-pods",
MountPath: "/host_pods",
MountPropagation: &mountPropagationToHostContainer,
},
{
Name: "scratch",
MountPath: "/scratch",
},
{
Name: "certs",
MountPath: "/etc/ssl/certs",
},
},
Env: []v1.EnvVar{
{
Name: "NODE_NAME",
ValueFrom: &v1.EnvVarSource{
FieldRef: &v1.ObjectFieldSelector{
FieldPath: "spec.nodeName",
},
},
},
{
Name: "VELERO_NAMESPACE",
ValueFrom: &v1.EnvVarSource{
FieldRef: &v1.ObjectFieldSelector{
FieldPath: "metadata.namespace",
},
},
},
{
Name: "VELERO_SCRATCH_DIR",
Value: "/scratch",
},
},
},
},
},
},
},
},
},
{
name: "Invalid podConfig label for velero and restic",
args: args{
&oadpv1alpha1.DataProtectionApplication{
Spec: oadpv1alpha1.DataProtectionApplicationSpec{
Configuration: &oadpv1alpha1.ApplicationConfig{
Restic: &oadpv1alpha1.ResticConfig{
PodConfig: &oadpv1alpha1.PodConfig{
Labels: map[string]string{
"name": "not-restic", // this label is already defined by https://github.com/openshift/velero/blob/198ea57407d5271dc4ae00068123754ecff306ea/pkg/install/daemonset.go#L72
},
},
},
Velero: &oadpv1alpha1.VeleroConfig{
PodConfig: &oadpv1alpha1.PodConfig{
Labels: map[string]string{
"veleroLabel": "this is a label",
},
},
},
},
},
}, &appsv1.DaemonSet{
ObjectMeta: getResticObjectMeta(r),
},
},
wantErr: true,
want: nil,
},
{
name: "test restic nodeselector customization via dpa",
args: args{
Expand Down
Loading