Merge pull request #13788 from coreydaley/trello_1044_cleanup_policy_…

…for_builds

Merged by openshift-bot

api/swagger-spec/oapi-v1.json

@@ -23359,6 +23359,16 @@
      "nodeSelector": {
       "type": "object",
       "description": "nodeSelector is a selector which must be true for the build pod to fit on a node If nil, it can be overridden by default build nodeselector values for the cluster. If set to an empty map or a map with any values, default build nodeselector values are ignored."
+     },
+     "successfulBuildsHistoryLimit": {
+      "type": "integer",
+      "format": "int32",
+      "description": "successfulBuildsHistoryLimit is the number of old successful builds to retain. If not specified, all successful builds are retained."
+     },
+     "failedBuildsHistoryLimit": {
+      "type": "integer",
+      "format": "int32",
+      "description": "failedBuildsHistoryLimit is the number of old failed builds to retain. If not specified, all failed builds are retained."
      }
     }
    },

api/swagger-spec/openshift-openapi-spec.json

@@ -72737,6 +72737,11 @@
       "type": "integer",
       "format": "int64"
      },
+     "failedBuildsHistoryLimit": {
+      "description": "failedBuildsHistoryLimit is the number of old failed builds to retain. If not specified, all failed builds are retained.",
+      "type": "integer",
+      "format": "int32"
+     },
      "nodeSelector": {
       "description": "nodeSelector is a selector which must be true for the build pod to fit on a node If nil, it can be overridden by default build nodeselector values for the cluster. If set to an empty map or a map with any values, default build nodeselector values are ignored.",
       "type": "object",
@@ -72776,6 +72781,11 @@
       "description": "strategy defines how to perform a build.",
       "$ref": "#/definitions/v1.BuildStrategy"
      },
+     "successfulBuildsHistoryLimit": {
+      "description": "successfulBuildsHistoryLimit is the number of old successful builds to retain. If not specified, all successful builds are retained.",
+      "type": "integer",
+      "format": "int32"
+     },
      "triggers": {
       "description": "triggers determine how new Builds can be launched from a BuildConfig. If no triggers are defined, a new build can only occur as a result of an explicit client build creation.",
       "type": "array",

pkg/build/api/types.go

@@ -991,6 +991,14 @@ type BuildConfigSpec struct {
 
 	// CommonSpec is the desired build specification
 	CommonSpec
+
+	// SuccessfulBuildsHistoryLimit is the number of old successful builds to retain.
+	// This field is a pointer to allow for differentiation between an explicit zero and not specified.
+	SuccessfulBuildsHistoryLimit *int32
+
+	// FailedBuildsHistoryLimit is the number of old failed builds to retain.
+	// This field is a pointer to allow for differentiation between an explicit zero and not specified.
+	FailedBuildsHistoryLimit *int32
 }
 
 // BuildRunPolicy defines the behaviour of how the new builds are executed

pkg/build/api/v1/swagger_doc.go

@@ -71,9 +71,11 @@ func (BuildConfigList) SwaggerDoc() map[string]string {
 }
 
 var map_BuildConfigSpec = map[string]string{
-	"":          "BuildConfigSpec describes when and how builds are created",
-	"triggers":  "triggers determine how new Builds can be launched from a BuildConfig. If no triggers are defined, a new build can only occur as a result of an explicit client build creation.",
-	"runPolicy": "RunPolicy describes how the new build created from this build configuration will be scheduled for execution. This is optional, if not specified we default to \"Serial\".",
+	"":                             "BuildConfigSpec describes when and how builds are created",
+	"triggers":                     "triggers determine how new Builds can be launched from a BuildConfig. If no triggers are defined, a new build can only occur as a result of an explicit client build creation.",
+	"runPolicy":                    "RunPolicy describes how the new build created from this build configuration will be scheduled for execution. This is optional, if not specified we default to \"Serial\".",
+	"successfulBuildsHistoryLimit": "successfulBuildsHistoryLimit is the number of old successful builds to retain. If not specified, all successful builds are retained.",
+	"failedBuildsHistoryLimit":     "failedBuildsHistoryLimit is the number of old failed builds to retain. If not specified, all failed builds are retained.",
 }
 
 func (BuildConfigSpec) SwaggerDoc() map[string]string {

pkg/build/api/v1/types.go

@@ -871,6 +871,14 @@ type BuildConfigSpec struct {
 
 	// CommonSpec is the desired build specification
 	CommonSpec `json:",inline" protobuf:"bytes,3,opt,name=commonSpec"`
+
+	// successfulBuildsHistoryLimit is the number of old successful builds to retain.
+	// If not specified, all successful builds are retained.
+	SuccessfulBuildsHistoryLimit *int32 `json:"successfulBuildsHistoryLimit,omitempty" protobuf:"varint,4,opt,name=successfulBuildsHistoryLimit"`
+
+	// failedBuildsHistoryLimit is the number of old failed builds to retain.
+	// If not specified, all failed builds are retained.
+	FailedBuildsHistoryLimit *int32 `json:"failedBuildsHistoryLimit,omitempty" protobuf:"varint,5,opt,name=failedBuildsHistoryLimit"`
 }
 
 // BuildRunPolicy defines the behaviour of how the new builds are executed

pkg/build/api/v1/zz_generated.conversion.go

@@ -308,6 +308,8 @@ func autoConvert_v1_BuildConfigSpec_To_api_BuildConfigSpec(in *BuildConfigSpec,
 	if err := Convert_v1_CommonSpec_To_api_CommonSpec(&in.CommonSpec, &out.CommonSpec, s); err != nil {
 		return err
 	}
+	out.SuccessfulBuildsHistoryLimit = (*int32)(unsafe.Pointer(in.SuccessfulBuildsHistoryLimit))
+	out.FailedBuildsHistoryLimit = (*int32)(unsafe.Pointer(in.FailedBuildsHistoryLimit))
 	return nil
 }
 
@@ -331,6 +333,8 @@ func autoConvert_api_BuildConfigSpec_To_v1_BuildConfigSpec(in *api.BuildConfigSp
 	if err := Convert_api_CommonSpec_To_v1_CommonSpec(&in.CommonSpec, &out.CommonSpec, s); err != nil {
 		return err
 	}
+	out.SuccessfulBuildsHistoryLimit = (*int32)(unsafe.Pointer(in.SuccessfulBuildsHistoryLimit))
+	out.FailedBuildsHistoryLimit = (*int32)(unsafe.Pointer(in.FailedBuildsHistoryLimit))
 	return nil
 }
 

pkg/build/api/v1/zz_generated.deepcopy.go

@@ -179,6 +179,16 @@ func DeepCopy_v1_BuildConfigSpec(in interface{}, out interface{}, c *conversion.
 		if err := DeepCopy_v1_CommonSpec(&in.CommonSpec, &out.CommonSpec, c); err != nil {
 			return err
 		}
+		if in.SuccessfulBuildsHistoryLimit != nil {
+			in, out := &in.SuccessfulBuildsHistoryLimit, &out.SuccessfulBuildsHistoryLimit
+			*out = new(int32)
+			**out = **in
+		}
+		if in.FailedBuildsHistoryLimit != nil {
+			in, out := &in.FailedBuildsHistoryLimit, &out.FailedBuildsHistoryLimit
+			*out = new(int32)
+			**out = **in
+		}
 		return nil
 	}
 }

pkg/build/api/validation/validation.go

@@ -148,6 +148,14 @@ func ValidateBuildConfig(config *buildapi.BuildConfig) field.ErrorList {
 			"run policy must Parallel, Serial, or SerialLatestOnly"))
 	}
 
+	if config.Spec.SuccessfulBuildsHistoryLimit != nil {
+		allErrs = append(allErrs, validation.ValidateNonnegativeField(int64(*config.Spec.SuccessfulBuildsHistoryLimit), specPath.Child("successfulBuildsHistoryLimit"))...)
+	}
+
+	if config.Spec.FailedBuildsHistoryLimit != nil {
+		allErrs = append(allErrs, validation.ValidateNonnegativeField(int64(*config.Spec.FailedBuildsHistoryLimit), specPath.Child("failedBuildsHistoryLimit"))...)
+	}
+
 	allErrs = append(allErrs, validateCommonSpec(&config.Spec.CommonSpec, specPath)...)
 
 	return allErrs

pkg/build/api/zz_generated.deepcopy.go

@@ -180,6 +180,16 @@ func DeepCopy_api_BuildConfigSpec(in interface{}, out interface{}, c *conversion
 		if err := DeepCopy_api_CommonSpec(&in.CommonSpec, &out.CommonSpec, c); err != nil {
 			return err
 		}
+		if in.SuccessfulBuildsHistoryLimit != nil {
+			in, out := &in.SuccessfulBuildsHistoryLimit, &out.SuccessfulBuildsHistoryLimit
+			*out = new(int32)
+			**out = **in
+		}
+		if in.FailedBuildsHistoryLimit != nil {
+			in, out := &in.FailedBuildsHistoryLimit, &out.FailedBuildsHistoryLimit
+			*out = new(int32)
+			**out = **in
+		}
 		return nil
 	}
 }

pkg/build/client/clients.go

@@ -8,7 +8,7 @@ import (
 
 // BuildConfigGetter provides methods for getting BuildConfigs
 type BuildConfigGetter interface {
-	Get(namespace, name string) (*buildapi.BuildConfig, error)
+	Get(namespace, name string, options metav1.GetOptions) (*buildapi.BuildConfig, error)
 }
 
 // BuildConfigUpdater provides methods for updating BuildConfigs
@@ -88,6 +88,29 @@ func (c OSClientBuildClonerClient) Clone(namespace string, request *buildapi.Bui
 	return c.Client.Builds(namespace).Clone(request)
 }
 
+// BuildDeleter knows how to delete builds from OpenShift.
+type BuildDeleter interface {
+	// DeleteBuild removes the build from OpenShift's storage.
+	DeleteBuild(build *buildapi.Build) error
+}
+
+// buildDeleter removes a build from OpenShift.
+type buildDeleter struct {
+	builds osclient.BuildsNamespacer
+}
+
+// NewBuildDeleter creates a new buildDeleter.
+func NewBuildDeleter(builds osclient.BuildsNamespacer) BuildDeleter {
+	return &buildDeleter{
+		builds: builds,
+	}
+}
+
+// DeleteBuild deletes a build from OpenShift.
+func (p *buildDeleter) DeleteBuild(build *buildapi.Build) error {
+	return p.builds.Builds(build.Namespace).Delete(build.Name)
+}
+
 // BuildConfigInstantiator provides methods for instantiating builds from build configs
 type BuildConfigInstantiator interface {
 	Instantiate(namespace string, request *buildapi.BuildRequest) (*buildapi.Build, error)

pkg/build/controller/build_controller.go

@@ -26,6 +26,8 @@ import (
 type BuildController struct {
 	BuildUpdater      buildclient.BuildUpdater
 	BuildLister       buildclient.BuildLister
+	BuildConfigGetter buildclient.BuildConfigGetter
+	BuildDeleter      buildclient.BuildDeleter
 	PodManager        podManager
 	BuildStrategy     BuildStrategy
 	ImageStreamClient imageStreamClient
@@ -85,7 +87,7 @@ func (bc *BuildController) CancelBuild(build *buildapi.Build) error {
 
 	glog.V(4).Infof("Build %s/%s was successfully cancelled.", build.Namespace, build.Name)
 
-	common.HandleBuildCompletion(build, bc.RunPolicies)
+	common.HandleBuildCompletion(build, bc.BuildLister, bc.BuildConfigGetter, bc.BuildDeleter, bc.RunPolicies)
 
 	return nil
 }

pkg/build/controller/build_controller_test.go

@@ -28,6 +28,24 @@ func (okc *okBuildLister) List(namespace string, opts metav1.ListOptions) (*buil
 	return &buildapi.BuildList{Items: []buildapi.Build{}}, nil
 }
 
+type okBuildDeleter struct{}
+
+func (okc *okBuildDeleter) DeleteBuild(*buildapi.Build) error {
+	return nil
+}
+
+type okBuildConfigGetter struct {
+	BuildConfig *buildapi.BuildConfig
+}
+
+func (okc *okBuildConfigGetter) Get(namespace, name string, options metav1.GetOptions) (*buildapi.BuildConfig, error) {
+	if okc.BuildConfig != nil {
+		return okc.BuildConfig, nil
+	} else {
+		return &buildapi.BuildConfig{}, nil
+	}
+}
+
 type errBuildUpdater struct{}
 
 func (ec *errBuildUpdater) Update(namespace string, build *buildapi.Build) error {
@@ -153,6 +171,8 @@ func mockBuildController() *BuildController {
 	return &BuildController{
 		BuildUpdater:      &okBuildUpdater{},
 		BuildLister:       &okBuildLister{},
+		BuildDeleter:      &okBuildDeleter{},
+		BuildConfigGetter: &okBuildConfigGetter{},
 		PodManager:        &okPodManager{},
 		BuildStrategy:     &okStrategy{},
 		ImageStreamClient: &okImageStreamClient{},

pkg/build/controller/buildconfig_controller.go

@@ -12,6 +12,7 @@ import (
 
 	buildapi "github.com/openshift/origin/pkg/build/api"
 	buildclient "github.com/openshift/origin/pkg/build/client"
+	buildutil "github.com/openshift/origin/pkg/build/controller/common"
 	buildgenerator "github.com/openshift/origin/pkg/build/generator"
 )
 
@@ -35,14 +36,20 @@ func IsFatal(err error) bool {
 
 type BuildConfigController struct {
 	BuildConfigInstantiator buildclient.BuildConfigInstantiator
-
+	BuildConfigGetter       buildclient.BuildConfigGetter
+	BuildLister             buildclient.BuildLister
+	BuildDeleter            buildclient.BuildDeleter
 	// recorder is used to record events.
 	Recorder record.EventRecorder
 }
 
 func (c *BuildConfigController) HandleBuildConfig(bc *buildapi.BuildConfig) error {
 	glog.V(4).Infof("Handling BuildConfig %s/%s", bc.Namespace, bc.Name)
 
+	if err := buildutil.HandleBuildPruning(bc.Name, bc.Namespace, c.BuildLister, c.BuildConfigGetter, c.BuildDeleter); err != nil {
+		utilruntime.HandleError(err)
+	}
+
 	hasChangeTrigger := buildapi.HasTriggerType(buildapi.ConfigChangeBuildTriggerType, bc)
 
 	if !hasChangeTrigger {

pkg/build/controller/buildconfig_controller_test.go

@@ -46,6 +46,9 @@ func TestHandleBuildConfig(t *testing.T) {
 		}
 		controller := &BuildConfigController{
 			BuildConfigInstantiator: instantiator,
+			BuildLister:             &okBuildLister{},
+			BuildDeleter:            &okBuildDeleter{},
+			BuildConfigGetter:       &okBuildConfigGetter{BuildConfig: tc.bc},
 			Recorder:                &record.FakeRecorder{},
 		}
 		err := controller.HandleBuildConfig(tc.bc)

pkg/build/controller/buildpod/buildpod_controller.go

@@ -47,8 +47,11 @@ type BuildPodController struct {
 
 	queue workqueue.RateLimitingInterface
 
-	buildStore  oscache.StoreToBuildLister
-	podInformer kcoreinformers.PodInformer
+	buildStore        oscache.StoreToBuildLister
+	buildLister       buildclient.BuildLister
+	buildConfigGetter buildclient.BuildConfigGetter
+	buildDeleter      buildclient.BuildDeleter
+	podInformer       kcoreinformers.PodInformer
 
 	buildStoreSynced func() bool
 	podStoreSynced   func() bool
@@ -64,13 +67,19 @@ func NewBuildPodController(buildInformer cache.SharedIndexInformer, podInformer
 	eventBroadcaster.StartRecordingToSink(&v1core.EventSinkImpl{Interface: v1core.New(extkc.Core().RESTClient()).Events("")})
 
 	buildListerUpdater := buildclient.NewOSClientBuildClient(oc)
+	buildConfigGetter := buildclient.NewOSClientBuildConfigClient(oc)
+	buildDeleter := buildclient.NewBuildDeleter(oc)
+
 	c := &BuildPodController{
-		buildUpdater: buildListerUpdater,
-		secretClient: intkc.Core(), // TODO: Replace with cache client
-		podClient:    intkc.Core(),
-		podInformer:  podInformer,
-		queue:        workqueue.NewRateLimitingQueue(workqueue.DefaultControllerRateLimiter()),
-		recorder:     eventBroadcaster.NewRecorder(kapi.Scheme, clientv1.EventSource{Component: "build-pod-controller"}),
+		buildUpdater:      buildListerUpdater,
+		buildLister:       buildListerUpdater,
+		buildConfigGetter: buildConfigGetter,
+		buildDeleter:      buildDeleter,
+		secretClient:      intkc.Core(), // TODO: Replace with cache client
+		podClient:         intkc.Core(),
+		podInformer:       podInformer,
+		queue:             workqueue.NewRateLimitingQueue(workqueue.DefaultControllerRateLimiter()),
+		recorder:          eventBroadcaster.NewRecorder(kapi.Scheme, clientv1.EventSource{Component: "build-pod-controller"}),
 	}
 
 	c.runPolicies = policy.GetAllRunPolicies(buildListerUpdater, buildListerUpdater)
@@ -235,7 +244,7 @@ func (bc *BuildPodController) HandlePod(pod *kapi.Pod) error {
 		case buildapi.BuildPhaseError, buildapi.BuildPhaseFailed:
 			bc.recorder.Eventf(build, kapi.EventTypeNormal, buildapi.BuildFailedEventReason, fmt.Sprintf(buildapi.BuildFailedEventMessage, build.Namespace, build.Name))
 		}
-		common.HandleBuildCompletion(build, bc.runPolicies)
+		common.HandleBuildCompletion(build, bc.buildLister, bc.buildConfigGetter, bc.buildDeleter, bc.runPolicies)
 	}
 
 	return nil
@@ -281,7 +290,7 @@ func (bc *BuildPodController) HandleBuildPodDeletion(pod *kapi.Pod) error {
 		if err := bc.buildUpdater.Update(build.Namespace, build); err != nil {
 			return fmt.Errorf("Failed to update build %s/%s: %v", build.Namespace, build.Name, err)
 		}
-		common.HandleBuildCompletion(build, bc.runPolicies)
+		common.HandleBuildCompletion(build, bc.buildLister, bc.buildConfigGetter, bc.buildDeleter, bc.runPolicies)
 	}
 	return nil
 }