fix(image): fix image policy default is 'Always' (#161)

pkg/builder/container.go

@@ -4,28 +4,29 @@ import (
  "slices"
 
  commonsv1alpha1 "github.com/zncdatadev/operator-go/pkg/apis/commons/v1alpha1"
+ "github.com/zncdatadev/operator-go/pkg/util"
  corev1 "k8s.io/api/core/v1"
  "k8s.io/apimachinery/pkg/util/intstr"
 )
 
 var (
  HTTPGetProbHandler2PortNames = []string{"http", "ui", "metrics", "health"}
  TCPProbHandler2PortNames = []string{"master"}
- DefaultImagePullPolicy = &[]corev1.PullPolicy{corev1.PullAlways}[0]
 )
 
 var _ ContainerBuilder = &Container{}
 
 type Container struct {
  Name string
- Image string
+ Image *util.Image
 
  obj *corev1.Container
 }
 
 // NewContainer returns a new Container
 func NewContainer(
- name, image string,
+ name string,
+ image *util.Image,
 ) *Container {
  return &Container{
  Name: name,
@@ -37,11 +38,13 @@ func NewContainer(
 // This method return a ContainerBuilder interface
 // Example:
 //
+// image := util.Image{Custom: "nginx"}
 // fooContainer := builder.NewContainerBuilder("foo", "nginx").
 // SetImagePullPolicy(corev1.PullAlways).
 // Build()
 func NewContainerBuilder(
- name, image string,
+ name string,
+ image *util.Image,
 ) ContainerBuilder {
  return NewContainer(name, image)
 }
@@ -50,8 +53,8 @@ func (b *Container) getObject() *corev1.Container {
  if b.obj == nil {
  b.obj = &corev1.Container{
  Name: b.Name,
- Image: b.Image,
- ImagePullPolicy: corev1.PullAlways,
+ Image: b.Image.String(),
+ ImagePullPolicy: b.Image.GetPullPolicy(),
  Resources: corev1.ResourceRequirements{},
  }
  }
@@ -63,13 +66,13 @@ func (b *Container) Build() *corev1.Container {
  return obj
 }
 
-func (b *Container) SetImagePullPolicy(policy *corev1.PullPolicy) ContainerBuilder {
- if policy == nil {
- logger.V(2).Info("Could not set image pull policy, use default value", "policy", policy, "container", b.Name, "image", b.Image, "default", DefaultImagePullPolicy)
+func (b *Container) SetImagePullPolicy(policy corev1.PullPolicy) ContainerBuilder {
+ if policy == "" {
+ logger.V(2).Info("Could not set image pull policy, use default value", "policy", policy, "container", b.Name, "image", b.Image, "default", b.Image.GetPullPolicy())
  return b
  }
 
- b.getObject().ImagePullPolicy = *policy
+ b.getObject().ImagePullPolicy = policy
  return b
 }
 

pkg/builder/deployment_test.go

@@ -27,8 +27,7 @@ type TrinoDeploymentBuilder struct {
 }
 
 func (b *TrinoDeploymentBuilder) Build(ctx context.Context) (ctrlclient.Object, error) {
-
- fooContainer := builder.NewContainerBuilder("coordinator", b.GetImageWithTag()).
+ fooContainer := builder.NewContainerBuilder("coordinator", b.GetImage()).
  SetCommand([]string{"bin/launcher"}).
  SetArgs([]string{"run"}).
  AddEnvVar(&corev1.EnvVar{

pkg/builder/interfaces.go

@@ -20,7 +20,7 @@ import (
 type ContainerBuilder interface {
  Build() *corev1.Container
 
- SetImagePullPolicy(policy *corev1.PullPolicy) ContainerBuilder
+ SetImagePullPolicy(policy corev1.PullPolicy) ContainerBuilder
 
  AddVolumeMounts(mounts []corev1.VolumeMount) ContainerBuilder
  AddVolumeMount(mount *corev1.VolumeMount) ContainerBuilder
@@ -65,7 +65,7 @@ type ResourceBuilder interface {
 type WorkloadImage interface {
  SetImage(image *util.Image)
  GetImage() *util.Image
- GetImageWithTag() string
+ GetImageWithTag() (string, error)
 }
 
 type WorkloadSecurityContext interface {

pkg/builder/vector.go

@@ -178,8 +178,7 @@ func (v *VectorDecorator) appendVectorContainer(containers *[]corev1.Container)
 }
 
 func (v *VectorDecorator) NewVectorContainer() *corev1.Container {
- vectorContainer := NewContainerBuilder(VectorContainerName, v.Image.String()).
- SetImagePullPolicy(v.Image.GetPullPolicy()).
+ vectorContainer := NewContainerBuilder(VectorContainerName, v.Image).
  SetCommand(VectorCommand()).
  SetArgs(VectorCommandArgs()).
  AddVolumeMounts(VectorVolumeMount(v.VectorConfigVolumeName, v.LogVolumeName)).

pkg/builder/workload.go

@@ -88,7 +88,7 @@ func (b *BaseWorkloadBuilder) GetImage() *util.Image {
  return b.image
 }
 
-func (b *BaseWorkloadBuilder) GetImageWithTag() string {
+func (b *BaseWorkloadBuilder) GetImageWithTag() (string, error) {
  return b.image.GetImageWithTag()
 }
 

pkg/reconciler/deployment_test.go

@@ -31,7 +31,7 @@ type TrinoCoordinatorDeploymentBuilder struct {
 }
 
 func (b *TrinoCoordinatorDeploymentBuilder) Build(ctx context.Context) (ctrlclient.Object, error) {
- trinoContainer := builder.NewContainerBuilder("coordinator", b.GetImageWithTag()).
+ trinoContainer := builder.NewContainerBuilder("coordinator", b.GetImage()).
  SetCommand([]string{"/usr/lib/trino/bin/launcher", "run"}).
  SetImagePullPolicy(b.GetImage().PullPolicy).
  Build()
@@ -130,7 +130,7 @@ var _ = Describe("Deloyment reconciler", func() {
  Expect(k8sClient.Get(ctx, ctrlclient.ObjectKey{Namespace: namespace, Name: name}, deployment)).Should(Succeed())
  Expect(deployment.Spec.Template.Spec.Containers).ShouldNot(BeNil())
  Expect(deployment.Spec.Template.Spec.Containers).Should(HaveLen(1))
- Expect(deployment.Spec.Template.Spec.Containers[0].ImagePullPolicy).Should(Equal(*builder.DefaultImagePullPolicy))
+ Expect(deployment.Spec.Template.Spec.Containers[0].ImagePullPolicy).Should(Equal(util.DefaultImagePullPolicy))
  })
 
  It("Should successfully reconcile deployment replicas to 0 when stopped", func() {

pkg/reconciler/statefulset_test.go

@@ -29,7 +29,7 @@ type FooStatefulSetBuilder struct {
 func (b *FooStatefulSetBuilder) Build(ctx context.Context) (ctrlclient.Object, error) {
  containerBuilder := builder.NewContainer(
  "foo",
- "nginx",
+ b.GetImage(),
  )
 
  b.AddContainer(containerBuilder.Build())

pkg/util/image.go

@@ -1,10 +1,13 @@
 package util
 
 import (
+ "fmt"
+
  corev1 "k8s.io/api/core/v1"
 )
 
 var DefaultRepository = "quay.io/zncdatadev"
+var DefaultImagePullPolicy = corev1.PullIfNotPresent
 
 // TODO: add semver validation for version fields
 
@@ -22,7 +25,7 @@ type Image struct {
  ProductName string
  PlatformVersion string
  ProductVersion string
- PullPolicy *corev1.PullPolicy
+ PullPolicy corev1.PullPolicy
  PullSecretName string
 }
 
@@ -31,7 +34,7 @@ type ImageOption func(*ImageOptions)
 type ImageOptions struct {
  Custom string
  Repo string
- PullPolicy *corev1.PullPolicy
+ PullPolicy corev1.PullPolicy
  PullSecretName string
 }
 
@@ -77,33 +80,37 @@ func NewImage(
  }
 }
 
-func (i *Image) GetImageWithTag() string {
+func (i *Image) GetImageWithTag() (string, error) {
  if i.Custom != "" {
- return i.Custom
+ return i.Custom, nil
  }
 
  if i.ProductVersion == "" {
- panic("ProductVersion is required")
+ return "", fmt.Errorf("ProductVersion is required in Image")
  }
 
  if i.PlatformVersion == "" {
- panic("PlatformVersion is required")
+ return "", fmt.Errorf("PlatformVersion is required in Image")
  }
 
  if i.Repo == "" {
  i.Repo = DefaultRepository
  }
-
- return i.Repo + "/" + i.ProductName + ":" + i.ProductVersion + "-" + "kubedoop" + i.PlatformVersion
+ // quay.io/zncdatadev/myproduct:1.0.0-kubedoop1.0
+ return fmt.Sprintf("%s/%s:%s-kubedoop%s", i.Repo, i.ProductName, i.ProductVersion, i.PlatformVersion), nil
 }
 
 func (i *Image) String() string {
- return i.GetImageWithTag()
+ tag, err := i.GetImageWithTag()
+ if err != nil {
+ panic(err)
+ }
+ return tag
 }
 
-func (i *Image) GetPullPolicy() *corev1.PullPolicy {
- if i.PullPolicy == nil {
- return func() *corev1.PullPolicy { p := corev1.PullIfNotPresent; return &p }()
+func (i *Image) GetPullPolicy() corev1.PullPolicy {
+ if i.PullPolicy == "" {
+ return DefaultImagePullPolicy
  }
  return i.PullPolicy
 }

pkg/util/image_test.go

@@ -14,7 +14,11 @@ var _ = Describe("GetImageTag", func() {
  )
 
  BeforeEach(func() {
- tag = func() string { return image.GetImageWithTag() }
+ tag = func() string {
+ image, err := image.GetImageWithTag()
+ Expect(err).ShouldNot(HaveOccurred())
+ return image
+ }
  })
 
  It("should return the custom tag if provided", func() {
@@ -50,26 +54,26 @@ var _ = Describe("GetImageTag", func() {
 var _ = Describe("GetPullPolicy", func() {
  var (
  image util.Image
- policy func() *v1.PullPolicy
+ policy func() v1.PullPolicy
  )
 
  BeforeEach(func() {
- policy = func() *v1.PullPolicy {
+ policy = func() v1.PullPolicy {
  return image.GetPullPolicy()
  }
  })
 
  It("should return the existing PullPolicy when it is not nil", func() {
  pullPolicy := v1.PullAlways
  image = util.Image{
- PullPolicy: &pullPolicy,
+ PullPolicy: pullPolicy,
  }
- Expect(policy()).Should(Equal(&pullPolicy))
+ Expect(policy()).Should(Equal(pullPolicy))
  })
 
  It("should return PullIfNotPresent when PullPolicy is nil", func() {
  image = util.Image{}
  expected := v1.PullIfNotPresent
- Expect(policy()).Should(Equal(&expected))
+ Expect(policy()).Should(Equal(expected))
  })
 })