Skip to content

Commit

Permalink
Reduce CRD size.
Browse files Browse the repository at this point in the history
1. Make the Restore hook.InitConatianer server side field pruing disable.
2. Remove restore patch in update-generate-crd-code.sh.
3. Modify related testcases.
4. Add Container fields validation in Restore Init hook.

Signed-off-by: Xun Jiang <[email protected]>
  • Loading branch information
Xun Jiang committed Aug 2, 2022
1 parent 108c81d commit e8da5df
Show file tree
Hide file tree
Showing 13 changed files with 207 additions and 1,543 deletions.
1 change: 1 addition & 0 deletions changelogs/unreleased/5174-jxun
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Reduce CRD size.
1,421 changes: 3 additions & 1,418 deletions config/crd/v1/bases/velero.io_restores.yaml

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion config/crd/v1/crds/crds.go

Large diffs are not rendered by default.

3 changes: 0 additions & 3 deletions hack/restore-crd-patch-v1.json

This file was deleted.

3 changes: 0 additions & 3 deletions hack/restore-crd-patch-v1beta1.json

This file was deleted.

11 changes: 1 addition & 10 deletions hack/update-generated-crd-code.sh
Original file line number Diff line number Diff line change
Expand Up @@ -47,21 +47,12 @@ ${GOPATH}/src/k8s.io/code-generator/generate-groups.sh \
# Generate apiextensions.k8s.io/v1
# Generate manifests e.g. CRD, RBAC etc.
controller-gen \
crd:crdVersions=v1\
crd:crdVersions=v1 \
paths=./pkg/apis/velero/v1/... \
rbac:roleName=velero-perms \
paths=./pkg/controller/... \
output:crd:artifacts:config=config/crd/v1/bases \
object \
paths=./pkg/apis/velero/v1/...

# this is a super hacky workaround for https://github.com/kubernetes/kubernetes/issues/91395
# which a result of fixing the validation on CRD objects. The validation ensures the fields that are list map keys, are either marked
# as required or have default values to ensure merging of list map items work as expected.
# With "containerPort" and "protocol" being considered as x-kubernetes-list-map-keys in the container ports, and "protocol" was not
# a required field, the CRD would fail validation with errors similar to the one reported in https://github.com/kubernetes/kubernetes/issues/91395.
# once controller-gen (above) is able to generate CRDs with `protocol` as a required field, this hack can be removed.
kubectl patch -f config/crd/v1/bases/velero.io_restores.yaml -p "$(cat hack/restore-crd-patch-v1.json)" --type=json --local=true -o yaml > /tmp/velero.io_restores-yaml.patched
mv /tmp/velero.io_restores-yaml.patched config/crd/v1/bases/velero.io_restores.yaml

go generate ./config/crd/v1/crds
55 changes: 41 additions & 14 deletions internal/hook/item_hook_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,10 +131,10 @@ func (i *InitContainerRestoreHookHandler) HandleRestoreHooks(
pod.Spec.InitContainers = pod.Spec.InitContainers[1:]
}

hooksFromAnnotations := getInitRestoreHookFromAnnotation(kube.NamespaceAndName(pod), metadata.GetAnnotations(), log)
if hooksFromAnnotations != nil {
initContainerFromAnnotations := getInitContainerFromAnnotation(kube.NamespaceAndName(pod), metadata.GetAnnotations(), log)
if initContainerFromAnnotations != nil {
log.Infof("Handling InitRestoreHooks from pod annotations")
initContainers = append(initContainers, hooksFromAnnotations.InitContainers...)
initContainers = append(initContainers, *initContainerFromAnnotations)
} else {
log.Infof("Handling InitRestoreHooks from RestoreSpec")
// pod did not have the annotations appropriate for restore hooks
Expand All @@ -155,7 +155,22 @@ func (i *InitContainerRestoreHookHandler) HandleRestoreHooks(
}
for _, hook := range rh.RestoreHooks {
if hook.Init != nil {
initContainers = append(initContainers, hook.Init.InitContainers...)
containers := make([]corev1api.Container, 0)
for _, raw := range hook.Init.InitContainers {
container := corev1api.Container{}
err := ValidateContainer(raw.Raw)
if err != nil {
log.Errorf("invalid Restore Init hook: %s", err.Error())
return nil, err
}
err = json.Unmarshal(raw.Raw, &container)
if err != nil {
log.Errorf("fail to Unmarshal hook Init into container: %s", err.Error())
return nil, errors.WithStack(err)
}
containers = append(containers, container)
}
initContainers = append(initContainers, containers...)
}
}
}
Expand Down Expand Up @@ -350,7 +365,7 @@ type ResourceRestoreHook struct {
RestoreHooks []velerov1api.RestoreResourceHook
}

func getInitRestoreHookFromAnnotation(podName string, annotations map[string]string, log logrus.FieldLogger) *velerov1api.InitRestoreHook {
func getInitContainerFromAnnotation(podName string, annotations map[string]string, log logrus.FieldLogger) *corev1api.Container {
containerImage := annotations[podRestoreHookInitContainerImageAnnotationKey]
containerName := annotations[podRestoreHookInitContainerNameAnnotationKey]
command := annotations[podRestoreHookInitContainerCommandAnnotationKey]
Expand All @@ -373,15 +388,13 @@ func getInitRestoreHookFromAnnotation(podName string, annotations map[string]str
log.Infof("Pod %s has no %s annotation, using generated name %s for initContainer", podName, podRestoreHookInitContainerNameAnnotationKey, containerName)
}

return &velerov1api.InitRestoreHook{
InitContainers: []corev1api.Container{
{
Image: containerImage,
Name: containerName,
Command: parseStringToCommand(command),
},
},
initContainer := corev1api.Container{
Image: containerImage,
Name: containerName,
Command: parseStringToCommand(command),
}

return &initContainer
}

// GetRestoreHooksFromSpec returns a list of ResourceRestoreHooks from the restore Spec.
Expand All @@ -406,7 +419,7 @@ func GetRestoreHooksFromSpec(hooksSpec *velerov1api.RestoreHooks) ([]ResourceRes
if rs.LabelSelector != nil {
ls, err := metav1.LabelSelectorAsSelector(rs.LabelSelector)
if err != nil {
return nil, errors.WithStack(err)
return []ResourceRestoreHook{}, errors.WithStack(err)
}
rh.Selector.LabelSelector = ls
}
Expand Down Expand Up @@ -526,3 +539,17 @@ func GroupRestoreExecHooks(

return byContainer, nil
}

// ValidateContainer validate whether a map contains mandatory k8s Container fields.
// mandatory fields include name, image and commands.
func ValidateContainer(raw []byte) error {
container := corev1api.Container{}
err := json.Unmarshal(raw, &container)
if err != nil {
return err
}
if len(container.Command) <= 0 || len(container.Name) <= 0 || len(container.Image) <= 0 {
return fmt.Errorf("invalid InitContainer in restore hook, it doesn't have Command, Name or Image field")
}
return nil
}
Loading

0 comments on commit e8da5df

Please sign in to comment.