Updates success-build-history-limit and failed-build-history-limit …

…description, adds both commands to `patch`

Signed-off-by: Caroline Scherf <[email protected]>

docs/kp_image_create.md

@@ -62,7 +62,7 @@ kp image create my-image --tag my-registry.com/my-repo --blob https://my-blob-ho
                                                 resource with generated container image references. A "kubectl apply -f" of the
                                                 resource from --output without image uploads will result in a reconcile failure.
   -e, --env stringArray                       build time environment variables
-      --failed-build-history-limit string     set the failedBuildHistoryLimit
+      --failed-build-history-limit string     number of failed builds to keep, leave empty to use cluster default
       --git string                            git repository url
       --git-revision string                   git revision such as commit, tag, or branch (default "main")
   -h, --help                                  help for create
@@ -78,7 +78,7 @@ kp image create my-image --tag my-registry.com/my-repo --blob https://my-blob-ho
       --service-account string                service account name to use (default "default")
   -s, --service-binding stringArray           build time service bindings
       --sub-path string                       build code at the sub path located within the source code directory
-      --success-build-history-limit string    set the successBuildHistoryLimit
+      --success-build-history-limit string    number of successful builds to keep, leave empty to use cluster default
   -t, --tag string                            registry location where the OCI image will be created
   -w, --wait                                  wait for image create to be reconciled and tail resulting build logs
 ```

docs/kp_image_patch.md

@@ -65,7 +65,7 @@ kp image patch my-image --tag my-registry.com/my-repo --blob https://my-blob-hos
                                                resource with generated container image references. A "kubectl apply -f" of the
                                                resource from --output without image uploads will result in a reconcile failure.
   -e, --env stringArray                      build time environment variables to add/replace
-      --failed-build-history-limit string    set the failedBuildHistoryLimit
+      --failed-build-history-limit string    number of failed builds to keep, leave empty to use cluster default
       --git string                           git repository url
       --git-revision string                  git revision such as commit, tag, or branch (default "main")
   -h, --help                                 help for patch
@@ -80,7 +80,7 @@ kp image patch my-image --tag my-registry.com/my-repo --blob https://my-blob-hos
       --service-account string               service account name to use
   -s, --service-binding stringArray          build time service bindings to add/replace
       --sub-path string                      build code at the sub path located within the source code directory
-      --success-build-history-limit string   set the successBuildHistoryLimit
+      --success-build-history-limit string   number of successful builds to keep, leave empty to use cluster default
   -w, --wait                                 wait for image resource patch to be reconciled and tail resulting build logs
 ```
 

docs/kp_image_save.md

@@ -68,7 +68,7 @@ kp image save my-image --tag my-registry.com/my-repo --blob https://my-blob-host
                                                 resource with generated container image references. A "kubectl apply -f" of the
                                                 resource from --output without image uploads will result in a reconcile failure.
   -e, --env stringArray                       build time environment variables
-      --failed-build-history-limit string     set the failedBuildHistoryLimit
+      --failed-build-history-limit string     number of failed builds to keep, leave empty to use cluster default
       --git string                            git repository url
       --git-revision string                   git revision such as commit, tag, or branch (default "main")
   -h, --help                                  help for save
@@ -84,7 +84,7 @@ kp image save my-image --tag my-registry.com/my-repo --blob https://my-blob-host
       --service-account string                service account name to use
   -s, --service-binding stringArray           build time service bindings to add/replace
       --sub-path string                       build code at the sub path located within the source code directory
-      --success-build-history-limit string    set the successBuildHistoryLimit
+      --success-build-history-limit string    number of successful builds to keep, leave empty to use cluster default
   -t, --tag string                            registry location where the image will be created
   -w, --wait                                  wait for image create to be reconciled and tail resulting build logs
 ```

pkg/commands/image/create.go

@@ -108,8 +108,8 @@ kp image create my-image --tag my-registry.com/my-repo --blob https://my-blob-ho
 	cmd.Flags().StringArrayVarP(&factory.Env, "env", "e", []string{}, "build time environment variables")
 	cmd.Flags().StringArrayVarP(&factory.ServiceBinding, "service-binding", "s", []string{}, "build time service bindings")
 	cmd.Flags().StringVar(&factory.CacheSize, "cache-size", "", "cache size as a kubernetes quantity (default \"2G\")")
-	cmd.Flags().StringVar(&factory.SuccessBuildHistoryLimit, "success-build-history-limit", "", "set the successBuildHistoryLimit")
-	cmd.Flags().StringVar(&factory.FailedBuildHistoryLimit, "failed-build-history-limit", "", "set the failedBuildHistoryLimit")
+	cmd.Flags().StringVar(&factory.SuccessBuildHistoryLimit, "success-build-history-limit", "", "number of successful builds to keep, leave empty to use cluster default")
+	cmd.Flags().StringVar(&factory.FailedBuildHistoryLimit, "failed-build-history-limit", "", "number of failed builds to keep, leave empty to use cluster default")
 	cmd.Flags().StringVar(&factory.ServiceAccount, "service-account", "default", "service account name to use")
 	cmd.Flags().BoolP("wait", "w", false, "wait for image create to be reconciled and tail resulting build logs")
 	commands.SetImgUploadDryRunOutputFlags(cmd)

pkg/commands/image/patch.go

@@ -118,8 +118,8 @@ kp image patch my-image --tag my-registry.com/my-repo --blob https://my-blob-hos
 	cmd.Flags().StringArrayVarP(&factory.ServiceBinding, "service-binding", "s", []string{}, "build time service bindings to add/replace")
 	cmd.Flags().StringArrayVarP(&factory.DeleteServiceBinding, "delete-service-binding", "", []string{}, "build time service bindings to remove")
 	cmd.Flags().StringVar(&factory.CacheSize, "cache-size", "", "cache size as a kubernetes quantity")
-	cmd.Flags().StringVar(&factory.SuccessBuildHistoryLimit, "success-build-history-limit", "", "set the successBuildHistoryLimit")
-	cmd.Flags().StringVar(&factory.FailedBuildHistoryLimit, "failed-build-history-limit", "", "set the failedBuildHistoryLimit")
+	cmd.Flags().StringVar(&factory.SuccessBuildHistoryLimit, "success-build-history-limit", "", "number of successful builds to keep, leave empty to use cluster default")
+	cmd.Flags().StringVar(&factory.FailedBuildHistoryLimit, "failed-build-history-limit", "", "number of failed builds to keep, leave empty to use cluster default")
 	cmd.Flags().StringVar(&factory.ServiceAccount, "service-account", "", "service account name to use")
 	cmd.Flags().BoolP("wait", "w", false, "wait for image resource patch to be reconciled and tail resulting build logs")
 	commands.SetImgUploadDryRunOutputFlags(cmd)

pkg/commands/image/patch_test.go

@@ -121,6 +121,26 @@ Image Resource "some-image" patched
 			assert.Len(t, fakeImageWaiter.Calls, 0)
 		})
 
+		it("can patch the SuccessBuildHistoryLimit and FailedBuildHistoryLimit", func() {
+			testhelpers.CommandTest{
+				Objects: []runtime.Object{
+					existingImage,
+				},
+				Args: []string{
+					"some-image",
+					"--success-build-history-limit", "5",
+					"--failed-build-history-limit", "2",
+				},
+				ExpectedOutput: `Patching Image Resource...
+Image Resource "some-image" patched
+`,
+				ExpectPatches: []string{
+					`{"spec":{"failedBuildHistoryLimit":2,"successBuildHistoryLimit":5}}`,
+				},
+			}.TestKpack(t, cmdFunc)
+			assert.Len(t, fakeImageWaiter.Calls, 0)
+		})
+
 		when("patching source", func() {
 			when("patching the sub path", func() {
 				it("can patch it with an empty string", func() {

pkg/commands/image/save.go

@@ -129,8 +129,8 @@ kp image save my-image --tag my-registry.com/my-repo --blob https://my-blob-host
 	cmd.Flags().StringVar(&factory.LocalPathDestinationImage, "local-path-destination-image", "", "registry location of where the local source code will be uploaded to (default \"<image-tag-repo>-source\")")
 	cmd.Flags().StringVar(&subPath, "sub-path", "", "build code at the sub path located within the source code directory")
 	cmd.Flags().StringVar(&factory.CacheSize, "cache-size", "", "cache size as a kubernetes quantity (default \"2G\")")
-	cmd.Flags().StringVar(&factory.SuccessBuildHistoryLimit, "success-build-history-limit", "", "set the successBuildHistoryLimit")
-	cmd.Flags().StringVar(&factory.FailedBuildHistoryLimit, "failed-build-history-limit", "", "set the failedBuildHistoryLimit")
+	cmd.Flags().StringVar(&factory.SuccessBuildHistoryLimit, "success-build-history-limit", "", "number of successful builds to keep, leave empty to use cluster default")
+	cmd.Flags().StringVar(&factory.FailedBuildHistoryLimit, "failed-build-history-limit", "", "number of failed builds to keep, leave empty to use cluster default")
 	cmd.Flags().StringVarP(&factory.Builder, "builder", "b", "", "builder name")
 	cmd.Flags().StringVarP(&factory.ClusterBuilder, "cluster-builder", "c", "", "cluster builder name")
 	cmd.Flags().StringArrayVarP(&factory.Env, "env", "e", []string{}, "build time environment variables")

pkg/image/factory.go

@@ -124,20 +124,13 @@ func (f *Factory) MakeImage(name, namespace, tag string) (*v1alpha2.Image, error
 		}
 	}
 
-	if f.SuccessBuildHistoryLimit != "" {
-		successBuildHistoryLimit, err := strconv.ParseInt(f.SuccessBuildHistoryLimit, 10, 64)
-		if successBuildHistoryLimit < 1 || err != nil {
-			return nil, errors.New("must provide a valid success-build-history-limit > 0")
-		}
-		image.Spec.SuccessBuildHistoryLimit = &successBuildHistoryLimit
+	image.Spec.SuccessBuildHistoryLimit, err = f.makeBuildHistoryLimit(f.SuccessBuildHistoryLimit)
+	if err != nil {
+		return nil, err
 	}
-
-	if f.FailedBuildHistoryLimit != "" {
-		failedBuildHistoryLimit, err := strconv.ParseInt(f.FailedBuildHistoryLimit, 10, 64)
-		if failedBuildHistoryLimit < 1 || err != nil {
-			return nil, errors.New("must provide a valid failed-build-history-limit > 0")
-		}
-		image.Spec.FailedBuildHistoryLimit = &failedBuildHistoryLimit
+	image.Spec.FailedBuildHistoryLimit, err = f.makeBuildHistoryLimit(f.FailedBuildHistoryLimit)
+	if err != nil {
+		return nil, err
 	}
 
 	return image, nil
@@ -249,7 +242,7 @@ func (f *Factory) makeBuildHistoryLimit(buildHistoryLimit string) (*int64, error
 
 	value, err := strconv.ParseInt(buildHistoryLimit, 10, 64)
 	if err != nil || value < 1 {
-		return nil, errors.New("must provide a valid build history limit >1")
+		return nil, errors.New("must provide a valid build history limit > 0")
 	}
 
 	return &value, nil

pkg/image/factory_test.go

@@ -86,7 +86,7 @@ func testImageFactory(t *testing.T, when spec.G, it spec.S) {
 			factory.FailedBuildHistoryLimit = "10"
 			factory.Blob = "some-blob"
 			_, err := factory.MakeImage("test-name", "test-namespace", "test-registry.io/test-image")
-			require.EqualError(t, err, "must provide a valid success-build-history-limit > 0")
+			require.EqualError(t, err, "must provide a valid build history limit > 0")
 		})
 	})
 

pkg/image/update_factory.go

@@ -45,6 +45,10 @@ func (f *Factory) UpdateImage(img *v1alpha2.Image) (*v1alpha2.Image, error) {
 	f.setBuilder(updatedImage)
 
 	f.setServiceAccount(updatedImage)
+	err = f.setBuildHistoryLimit(updatedImage)
+	if err != nil {
+		return nil, err
+	}
 
 	return updatedImage, nil
 }
@@ -345,3 +349,21 @@ func (f *Factory) setServiceAccount(image *v1alpha2.Image) {
 		image.Spec.ServiceAccountName = f.ServiceAccount
 	}
 }
+
+func (f *Factory) setBuildHistoryLimit(image *v1alpha2.Image) error {
+	var err error
+	if f.SuccessBuildHistoryLimit != "" {
+		image.Spec.SuccessBuildHistoryLimit, err = f.makeBuildHistoryLimit(f.SuccessBuildHistoryLimit)
+		if err != nil {
+			return err
+		}
+	}
+
+	if f.FailedBuildHistoryLimit != "" {
+		image.Spec.FailedBuildHistoryLimit, err = f.makeBuildHistoryLimit(f.FailedBuildHistoryLimit)
+		if err != nil {
+			return err
+		}
+	}
+	return nil
+}