Skip to content

Commit

Permalink
Modifying code to incorporate review comments.
Browse files Browse the repository at this point in the history
Signed-off-by: Gleb Aronsky <gleb.aronsky@windriver.com>
  • Loading branch information
Gleb Aronsky committed Feb 27, 2023
1 parent 36bbe2b commit 523fd40
Show file tree
Hide file tree
Showing 15 changed files with 173 additions and 78 deletions.
24 changes: 16 additions & 8 deletions api/openapi-spec/swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -4904,6 +4904,14 @@
"description": "FilesystemOverhead describes the space reserved for overhead when using Filesystem volumes. A value is between 0 and 1, if not defined it is 0.055 (5.5% overhead)",
"$ref": "#/definitions/v1beta1.FilesystemOverhead"
},
"imagePullSecrets": {
"description": "The imagePullSecrets used to pull the container images",
"type": "array",
"items": {
"default": {},
"$ref": "#/definitions/v1.LocalObjectReference"
}
},
"importProxy": {
"description": "ImportProxy contains importer pod proxy configuration.",
"$ref": "#/definitions/v1beta1.ImportProxy"
Expand Down Expand Up @@ -4950,6 +4958,14 @@
"description": "FilesystemOverhead describes the space reserved for overhead when using Filesystem volumes. A percentage value is between 0 and 1",
"$ref": "#/definitions/v1beta1.FilesystemOverhead"
},
"imagePullSecrets": {
"description": "The imagePullSecrets used to pull the container images",
"type": "array",
"items": {
"default": {},
"$ref": "#/definitions/v1.LocalObjectReference"
}
},
"importProxy": {
"description": "ImportProxy contains importer pod proxy configuration.",
"$ref": "#/definitions/v1beta1.ImportProxy"
Expand Down Expand Up @@ -5023,14 +5039,6 @@
"Never"
]
},
"imagePullSecrets": {
"description": "The imagePullSecrets to pull the container images",
"type": "array",
"items": {
"default": {},
"$ref": "#/definitions/v1.LocalObjectReference"
}
},
"infra": {
"description": "Rules on which nodes CDI infrastructure pods will be scheduled",
"default": {},
Expand Down
4 changes: 1 addition & 3 deletions cmd/cdi-controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (
"github.com/pkg/errors"
"github.com/prometheus/client_golang/prometheus"
"go.uber.org/zap/zapcore"
corev1 "k8s.io/api/core/v1"
networkingv1 "k8s.io/api/networking/v1"
extv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
"k8s.io/apimachinery/pkg/fields"
Expand Down Expand Up @@ -58,7 +57,6 @@ var (
uploadProxyServiceName string
configName string
pullPolicy string
imagePullSecrets []corev1.LocalObjectReference
verbose string
installerLabels map[string]string
log = logf.Log.WithName("controller")
Expand Down Expand Up @@ -240,7 +238,7 @@ func start(ctx context.Context, cfg *rest.Config) {
os.Exit(1)
}

if _, err := controller.NewCloneController(mgr, log, clonerImage, pullPolicy, imagePullSecrets, verbose, uploadClientCertGenerator, uploadServerBundleFetcher, getTokenPublicKey(), installerLabels); err != nil {
if _, err := controller.NewCloneController(mgr, log, clonerImage, pullPolicy, verbose, uploadClientCertGenerator, uploadServerBundleFetcher, getTokenPublicKey(), installerLabels); err != nil {
klog.Errorf("Unable to setup clone controller: %v", err)
os.Exit(1)
}
Expand Down
48 changes: 31 additions & 17 deletions pkg/apis/core/v1beta1/openapi_generated.go

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

14 changes: 8 additions & 6 deletions pkg/controller/clone-controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,15 +73,13 @@ type CloneReconciler struct {
image string
verbose string
pullPolicy string
imagePullSecrets []corev1.LocalObjectReference
installerLabels map[string]string
}

// NewCloneController creates a new instance of the config controller.
func NewCloneController(mgr manager.Manager,
log logr.Logger,
image, pullPolicy string,
imagePullSecrets []corev1.LocalObjectReference,
image, pullPolicy,
verbose string,
clientCertGenerator generator.CertGenerator,
serverCAFetcher fetcher.CertBundleFetcher,
Expand All @@ -96,7 +94,6 @@ func NewCloneController(mgr manager.Manager,
image: image,
verbose: verbose,
pullPolicy: pullPolicy,
imagePullSecrets: imagePullSecrets,
recorder: mgr.GetEventRecorderFor("clone-controller"),
clientCertGenerator: clientCertGenerator,
serverCAFetcher: serverCAFetcher,
Expand Down Expand Up @@ -262,7 +259,7 @@ func (r *CloneReconciler) reconcileSourcePod(sourcePod *corev1.Pod, targetPvc *c
return 2 * time.Second, nil
}

sourcePod, err := r.CreateCloneSourcePod(r.image, r.pullPolicy, r.imagePullSecrets, targetPvc, log)
sourcePod, err := r.CreateCloneSourcePod(r.image, r.pullPolicy, targetPvc, log)
// Check if pod has failed and, in that case, record an event with the error
if podErr := cc.HandleFailedPod(err, cc.CreateCloneSourcePodName(targetPvc), targetPvc, r.recorder, r.client); podErr != nil {
return 0, podErr
Expand Down Expand Up @@ -479,7 +476,7 @@ func (r *CloneReconciler) cleanup(pvc *corev1.PersistentVolumeClaim, log logr.Lo
}

// CreateCloneSourcePod creates our cloning src pod which will be used for out of band cloning to read the contents of the src PVC
func (r *CloneReconciler) CreateCloneSourcePod(image, pullPolicy string, imagePullSecrets []corev1.LocalObjectReference, pvc *corev1.PersistentVolumeClaim, log logr.Logger) (*corev1.Pod, error) {
func (r *CloneReconciler) CreateCloneSourcePod(image, pullPolicy string, pvc *corev1.PersistentVolumeClaim, log logr.Logger) (*corev1.Pod, error) {
exists, sourcePvcNamespace, sourcePvcName := ParseCloneRequestAnnotation(pvc)
if !exists {
return nil, errors.Errorf("bad CloneRequest Annotation")
Expand All @@ -500,6 +497,11 @@ func (r *CloneReconciler) CreateCloneSourcePod(image, pullPolicy string, imagePu
return nil, err
}

imagePullSecrets, err := cc.GetImagePullSecrets(r.client)
if err != nil {
return nil, err
}

workloadNodePlacement, err := cc.GetWorkloadNodePlacement(r.client)
if err != nil {
return nil, err
Expand Down
11 changes: 11 additions & 0 deletions pkg/controller/common/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,17 @@ func GetDefaultPodResourceRequirements(client client.Client) (*v1.ResourceRequir
return cdiconfig.Status.DefaultPodResourceRequirements, nil
}

// GetImagePullSecrets gets the imagePullSecrets needed to pull images from the cdi config
func GetImagePullSecrets(client client.Client) ([]corev1.LocalObjectReference, error) {
cdiconfig := &cdiv1.CDIConfig{}
if err := client.Get(context.TODO(), types.NamespacedName{Name: common.ConfigName}, cdiconfig); err != nil {
klog.Errorf("Unable to find CDI configuration, %v\n", err)
return nil, err
}

return cdiconfig.Status.ImagePullSecrets, nil
}

// AddVolumeDevices returns VolumeDevice slice with one block device for pods using PV with block volume mode
func AddVolumeDevices() []v1.VolumeDevice {
volumeDevices := []v1.VolumeDevice{
Expand Down
12 changes: 12 additions & 0 deletions pkg/controller/config-controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,10 @@ func (r *CDIConfigReconciler) Reconcile(_ context.Context, req reconcile.Request
return reconcile.Result{}, err
}

if err := r.reconcileImagePullSecrets(config); err != nil {
return reconcile.Result{}, err
}

if err := r.reconcileFilesystemOverhead(config); err != nil {
return reconcile.Result{}, err
}
Expand Down Expand Up @@ -232,6 +236,14 @@ func (r *CDIConfigReconciler) reconcileStorageClass(config *cdiv1.CDIConfig) err
return nil
}

func (r *CDIConfigReconciler) reconcileImagePullSecrets(config *cdiv1.CDIConfig) error {
if config.Spec.ImagePullSecrets != nil {
config.Status.ImagePullSecrets = config.Spec.ImagePullSecrets
}

return nil
}

func (r *CDIConfigReconciler) reconcileDefaultPodResourceRequirements(config *cdiv1.CDIConfig) error {
cpuLimit, _ := resource.ParseQuantity(defaultCPULimit)
memLimit, _ := resource.ParseQuantity(defaultMemLimit)
Expand Down
8 changes: 7 additions & 1 deletion pkg/controller/datavolume/clone-controller-base.go
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,11 @@ func (r *CloneReconcilerBase) createExpansionPod(pvc *corev1.PersistentVolumeCla
return nil, err
}

imagePullSecrets, err := cc.GetImagePullSecrets(r.client)
if err != nil {
return nil, err
}

workloadNodePlacement, err := cc.GetWorkloadNodePlacement(r.client)
if err != nil {
return nil, err
Expand Down Expand Up @@ -447,7 +452,8 @@ func (r *CloneReconcilerBase) createExpansionPod(pvc *corev1.PersistentVolumeCla
Args: []string{"-c", "echo", "'hello cdi'"},
},
},
RestartPolicy: corev1.RestartPolicyOnFailure,
ImagePullSecrets: imagePullSecrets,
RestartPolicy: corev1.RestartPolicyOnFailure,
Volumes: []corev1.Volume{
{
Name: cc.DataVolName,
Expand Down
11 changes: 8 additions & 3 deletions pkg/controller/import-controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ type importerPodArgs struct {
pvc *corev1.PersistentVolumeClaim
scratchPvcName *string
podResourceRequirements *corev1.ResourceRequirements
imagePullSecrets []corev1.LocalObjectReference
workloadNodePlacement *sdkapi.NodePlacement
vddkImageName *string
priorityClassName string
Expand Down Expand Up @@ -867,6 +868,8 @@ func createImporterPod(log logr.Logger, client client.Client, args *importerPodA
return nil, err
}

args.imagePullSecrets = cdiv1.CDIConfig{}.Spec.ImagePullSecrets

args.workloadNodePlacement, err = cc.GetWorkloadNodePlacement(client)
if err != nil {
return nil, err
Expand Down Expand Up @@ -971,6 +974,7 @@ func makeNodeImporterPodSpec(args *importerPodArgs) *corev1.Pod {
Tolerations: args.workloadNodePlacement.Tolerations,
Affinity: args.workloadNodePlacement.Affinity,
PriorityClassName: args.priorityClassName,
ImagePullSecrets: args.imagePullSecrets,
},
}

Expand Down Expand Up @@ -1003,7 +1007,7 @@ func makeNodeImporterPodSpec(args *importerPodArgs) *corev1.Pod {
args.podEnvVar.ep = "http://localhost:8100/disk.img"
args.podEnvVar.readyFile = "/shared/ready"
args.podEnvVar.doneFile = "/shared/done"
setImporterPodCommons(pod, args.podEnvVar, args.pvc, args.podResourceRequirements)
setImporterPodCommons(pod, args.podEnvVar, args.pvc, args.podResourceRequirements, args.imagePullSecrets)
pod.Spec.Containers[0].VolumeMounts = append(pod.Spec.Containers[0].VolumeMounts, corev1.VolumeMount{
MountPath: "/shared",
Name: "shared-volume",
Expand Down Expand Up @@ -1088,7 +1092,7 @@ func makeImporterPodSpec(args *importerPodArgs) *corev1.Pod {
},
}

setImporterPodCommons(pod, args.podEnvVar, args.pvc, args.podResourceRequirements)
setImporterPodCommons(pod, args.podEnvVar, args.pvc, args.podResourceRequirements, args.imagePullSecrets)

if args.scratchPvcName != nil {
pod.Spec.Containers[0].VolumeMounts = append(pod.Spec.Containers[0].VolumeMounts, corev1.VolumeMount{
Expand Down Expand Up @@ -1162,12 +1166,13 @@ func makeImporterPodSpec(args *importerPodArgs) *corev1.Pod {
return pod
}

func setImporterPodCommons(pod *corev1.Pod, podEnvVar *importPodEnvVar, pvc *corev1.PersistentVolumeClaim, podResourceRequirements *corev1.ResourceRequirements) {
func setImporterPodCommons(pod *corev1.Pod, podEnvVar *importPodEnvVar, pvc *corev1.PersistentVolumeClaim, podResourceRequirements *corev1.ResourceRequirements, imagePullSecrets []corev1.LocalObjectReference) {
if podResourceRequirements != nil {
for i := range pod.Spec.Containers {
pod.Spec.Containers[i].Resources = *podResourceRequirements
}
}
pod.Spec.ImagePullSecrets = imagePullSecrets

ownerUID := pvc.UID
if len(pvc.OwnerReferences) == 1 {
Expand Down
10 changes: 8 additions & 2 deletions pkg/controller/upload-controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -579,12 +579,17 @@ func (r *UploadReconciler) createUploadPod(args UploadPodArgs) (*v1.Pod, error)
return nil, err
}

imagePullSecrets, err := cc.GetImagePullSecrets(r.client)
if err != nil {
return nil, err
}

workloadNodePlacement, err := cc.GetWorkloadNodePlacement(r.client)
if err != nil {
return nil, err
}

pod := r.makeUploadPodSpec(args, podResourceRequirements, workloadNodePlacement)
pod := r.makeUploadPodSpec(args, podResourceRequirements, imagePullSecrets, workloadNodePlacement)
util.SetRecommendedLabels(pod, r.installerLabels, "cdi-controller")

if err := r.client.Get(context.TODO(), types.NamespacedName{Name: args.Name, Namespace: ns}, pod); err != nil {
Expand Down Expand Up @@ -730,7 +735,7 @@ func createUploadServiceNameFromPvcName(pvc string) string {
return naming.GetServiceNameFromResourceName(createUploadResourceName(pvc))
}

func (r *UploadReconciler) makeUploadPodSpec(args UploadPodArgs, resourceRequirements *v1.ResourceRequirements, workloadNodePlacement *sdkapi.NodePlacement) *v1.Pod {
func (r *UploadReconciler) makeUploadPodSpec(args UploadPodArgs, resourceRequirements *v1.ResourceRequirements, imagePullSecrets []v1.LocalObjectReference, workloadNodePlacement *sdkapi.NodePlacement) *v1.Pod {
requestImageSize, _ := cc.GetRequestedImageSize(args.PVC)
serviceName := naming.GetServiceNameFromResourceName(args.Name)
pod := &v1.Pod{
Expand Down Expand Up @@ -840,6 +845,7 @@ func (r *UploadReconciler) makeUploadPodSpec(args UploadPodArgs, resourceRequire
Tolerations: workloadNodePlacement.Tolerations,
Affinity: workloadNodePlacement.Affinity,
PriorityClassName: cc.GetPriorityClass(args.PVC),
ImagePullSecrets: imagePullSecrets,
},
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/operator/controller/cr-manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,8 @@ func (r *ReconcileCDI) getNamespacedArgs(cr *cdiv1.CDI) *cdinamespaced.FactoryAr
if cr.Spec.ImagePullPolicy != "" {
result.PullPolicy = string(cr.Spec.ImagePullPolicy)
}
if len(cr.Spec.ImagePullSecrets) > 0 {
result.ImagePullSecrets = cr.Spec.ImagePullSecrets
if cr.Spec.Config != nil && len(cr.Spec.Config.ImagePullSecrets) > 0 {
result.ImagePullSecrets = cr.Spec.Config.ImagePullSecrets
}
if cr.Spec.PriorityClass != nil && string(*cr.Spec.PriorityClass) != "" {
result.PriorityClassName = string(*cr.Spec.PriorityClass)
Expand Down
Loading

0 comments on commit 523fd40

Please sign in to comment.