-
Notifications
You must be signed in to change notification settings - Fork 571
feat: replace extractContent emptyDir+initContainers with image volumes #3806
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -39,7 +39,6 @@ type grpcCatalogSourceDecorator struct { | |||||||||||||||||||||||
| *v1alpha1.CatalogSource | ||||||||||||||||||||||||
| createPodAsUser int64 | ||||||||||||||||||||||||
| opmImage string | ||||||||||||||||||||||||
| utilImage string | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| type UpdateNotReadyErr struct { | ||||||||||||||||||||||||
|
|
@@ -144,7 +143,7 @@ func (s *grpcCatalogSourceDecorator) ServiceAccount() *corev1.ServiceAccount { | |||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| func (s *grpcCatalogSourceDecorator) Pod(serviceAccount *corev1.ServiceAccount, defaultPodSecurityConfig v1alpha1.SecurityConfig) (*corev1.Pod, error) { | ||||||||||||||||||||||||
| pod, err := Pod(s.CatalogSource, "registry-server", s.opmImage, s.utilImage, s.Spec.Image, serviceAccount, s.Labels(), s.Annotations(), 5, 10, s.createPodAsUser, defaultPodSecurityConfig) | ||||||||||||||||||||||||
| pod, err := Pod(s.CatalogSource, "registry-server", s.opmImage, s.Spec.Image, serviceAccount, s.Labels(), s.Annotations(), 5, 10, s.createPodAsUser, defaultPodSecurityConfig) | ||||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||||
| return nil, err | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
@@ -159,7 +158,6 @@ type GrpcRegistryReconciler struct { | |||||||||||||||||||||||
| SSAClient *controllerclient.ServerSideApplier | ||||||||||||||||||||||||
| createPodAsUser int64 | ||||||||||||||||||||||||
| opmImage string | ||||||||||||||||||||||||
| utilImage string | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| var _ RegistryReconciler = &GrpcRegistryReconciler{} | ||||||||||||||||||||||||
|
|
@@ -262,23 +260,26 @@ func (c *GrpcRegistryReconciler) currentPodsWithCorrectImageAndSpec(logger *logr | |||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| func correctImages(source grpcCatalogSourceDecorator, pod *corev1.Pod) bool { | ||||||||||||||||||||||||
| if len(pod.Spec.InitContainers) != 0 || len(pod.Spec.Containers) != 1 { | ||||||||||||||||||||||||
| return false | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| if pod.Spec.Containers[0].Image != source.CatalogSource.Spec.Image { | ||||||||||||||||||||||||
| return false | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| if source.CatalogSource.Spec.GrpcPodConfig != nil && source.CatalogSource.Spec.GrpcPodConfig.ExtractContent != nil { | ||||||||||||||||||||||||
| if len(pod.Spec.InitContainers) != 2 { | ||||||||||||||||||||||||
| if len(pod.Spec.Volumes) == 0 { | ||||||||||||||||||||||||
| return false | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| if len(pod.Spec.Containers) != 1 { | ||||||||||||||||||||||||
| return false | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| return pod.Spec.InitContainers[0].Image == source.utilImage && | ||||||||||||||||||||||||
| pod.Spec.InitContainers[1].Image == source.CatalogSource.Spec.Image && | ||||||||||||||||||||||||
| pod.Spec.Containers[0].Image == source.opmImage | ||||||||||||||||||||||||
| return pod.Spec.Volumes[0].Name == opmVolumeName && | ||||||||||||||||||||||||
| pod.Spec.Volumes[0].VolumeSource.Image != nil && | ||||||||||||||||||||||||
| pod.Spec.Volumes[0].VolumeSource.Image.Reference == source.opmImage | ||||||||||||||||||||||||
|
Comment on lines
+273
to
+275
|
||||||||||||||||||||||||
| return pod.Spec.Volumes[0].Name == opmVolumeName && | |
| pod.Spec.Volumes[0].VolumeSource.Image != nil && | |
| pod.Spec.Volumes[0].VolumeSource.Image.Reference == source.opmImage | |
| for _, volume := range pod.Spec.Volumes { | |
| if volume.Name != opmVolumeName { | |
| continue | |
| } | |
| return volume.VolumeSource.Image != nil && | |
| volume.VolumeSource.Image.Reference == source.opmImage | |
| } | |
| return false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a reasonable suggestion, but we are explicitly structuring the pod in a particular way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. The PR change essentially just reverts the code to what it was before we created the initContainers, so I think we've demonstrated that it's stable enough.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will cause a teardown/recreation of any existing CatalogSource pods in the cluster at the time of an upgrade, right?