Convert RestartPolicy to string for v1beta3.

Fixed #3607 and spiritually support kubernetes#5475
akram · Apr 4, 2015 · 00b6f24 · 00b6f24
1 parent a7d43b0
commit 00b6f24
Show file tree

Hide file tree

Showing 29 changed files with 184 additions and 258 deletions.
diff --git a/cmd/integration/integration.go b/cmd/integration/integration.go
@@ -627,7 +627,7 @@ func runServiceTest(client *client.Client) {
 					ImagePullPolicy: "PullIfNotPresent",
 				},
 			},
-			RestartPolicy: api.RestartPolicy{Always: &api.RestartPolicyAlways{}},
+			RestartPolicy: api.RestartPolicyAlways,
 			DNSPolicy:     api.DNSClusterFirst,
 		},
 		Status: api.PodStatus{

diff --git a/pkg/api/serialization_test.go b/pkg/api/serialization_test.go
@@ -162,7 +162,7 @@ func TestEncode_Ptr(t *testing.T) {
 			Labels: map[string]string{"name": "foo"},
 		},
 		Spec: api.PodSpec{
-			RestartPolicy: api.RestartPolicy{Always: &api.RestartPolicyAlways{}},
+			RestartPolicy: api.RestartPolicyAlways,
 			DNSPolicy:     api.DNSClusterFirst,
 		},
 	}

diff --git a/pkg/api/testing/fuzzer.go b/pkg/api/testing/fuzzer.go
@@ -161,8 +161,8 @@ func FuzzerFor(t *testing.T, version string, src rand.Source) *fuzz.Fuzzer {
 			*p = policies[c.Rand.Intn(len(policies))]
 		},
 		func(rp *api.RestartPolicy, c fuzz.Continue) {
-			// Exactly one of the fields should be set.
-			fuzzOneOf(c, &rp.Always, &rp.OnFailure, &rp.Never)
+			policies := []api.RestartPolicy{api.RestartPolicyAlways, api.RestartPolicyNever, api.RestartPolicyOnFailure}
+			*rp = policies[c.Rand.Intn(len(policies))]
 		},
 		func(vs *api.VolumeSource, c fuzz.Continue) {
 			// Exactly one of the fields should be set.

diff --git a/pkg/api/types.go b/pkg/api/types.go
@@ -515,23 +515,17 @@ type PodContainerInfo struct {
 	ContainerInfo PodInfo `json:"containerInfo"`
 }
 
-type RestartPolicyAlways struct{}
-
-// TODO(dchen1107): Define what kinds of failures should restart.
-// TODO(dchen1107): Decide whether to support policy knobs, and, if so, which ones.
-type RestartPolicyOnFailure struct{}
-
-type RestartPolicyNever struct{}
-
 // RestartPolicy describes how the container should be restarted.
 // Only one of the following restart policies may be specified.
 // If none of the following policies is specified, the default one
 // is RestartPolicyAlways.
-type RestartPolicy struct {
-	Always    *RestartPolicyAlways    `json:"always,omitempty"`
-	OnFailure *RestartPolicyOnFailure `json:"onFailure,omitempty"`
-	Never     *RestartPolicyNever     `json:"never,omitempty"`
-}
+type RestartPolicy string
+
+const (
+	RestartPolicyAlways    RestartPolicy = "Always"
+	RestartPolicyOnFailure RestartPolicy = "OnFailure"
+	RestartPolicyNever     RestartPolicy = "Never"
+)
 
 // PodList is a list of Pods.
 type PodList struct {

diff --git a/pkg/api/v1beta1/conversion.go b/pkg/api/v1beta1/conversion.go
@@ -1120,6 +1120,33 @@ func init() {
 			return nil
 		},
 
+		func(in *newer.RestartPolicy, out *RestartPolicy, s conversion.Scope) error {
+			switch *in {
+			case newer.RestartPolicyAlways:
+				*out = RestartPolicy{Always: &RestartPolicyAlways{}}
+			case newer.RestartPolicyNever:
+				*out = RestartPolicy{Never: &RestartPolicyNever{}}
+			case newer.RestartPolicyOnFailure:
+				*out = RestartPolicy{OnFailure: &RestartPolicyOnFailure{}}
+			default:
+				*out = RestartPolicy{}
+			}
+			return nil
+		},
+		func(in *RestartPolicy, out *newer.RestartPolicy, s conversion.Scope) error {
+			switch {
+			case in.Always != nil:
+				*out = newer.RestartPolicyAlways
+			case in.Never != nil:
+				*out = newer.RestartPolicyNever
+			case in.OnFailure != nil:
+				*out = newer.RestartPolicyOnFailure
+			default:
+				*out = ""
+			}
+			return nil
+		},
+
 		func(in *newer.Probe, out *LivenessProbe, s conversion.Scope) error {
 			if err := s.Convert(&in.Exec, &out.Exec, 0); err != nil {
 				return err

diff --git a/pkg/api/v1beta2/conversion.go b/pkg/api/v1beta2/conversion.go
@@ -1036,6 +1036,33 @@ func init() {
 			return nil
 		},
 
+		func(in *newer.RestartPolicy, out *RestartPolicy, s conversion.Scope) error {
+			switch *in {
+			case newer.RestartPolicyAlways:
+				*out = RestartPolicy{Always: &RestartPolicyAlways{}}
+			case newer.RestartPolicyNever:
+				*out = RestartPolicy{Never: &RestartPolicyNever{}}
+			case newer.RestartPolicyOnFailure:
+				*out = RestartPolicy{OnFailure: &RestartPolicyOnFailure{}}
+			default:
+				*out = RestartPolicy{}
+			}
+			return nil
+		},
+		func(in *RestartPolicy, out *newer.RestartPolicy, s conversion.Scope) error {
+			switch {
+			case in.Always != nil:
+				*out = newer.RestartPolicyAlways
+			case in.Never != nil:
+				*out = newer.RestartPolicyNever
+			case in.OnFailure != nil:
+				*out = newer.RestartPolicyOnFailure
+			default:
+				*out = ""
+			}
+			return nil
+		},
+
 		func(in *newer.Probe, out *LivenessProbe, s conversion.Scope) error {
 			if err := s.Convert(&in.Exec, &out.Exec, 0); err != nil {
 				return err

diff --git a/pkg/api/v1beta3/defaults.go b/pkg/api/v1beta3/defaults.go
@@ -52,11 +52,6 @@ func init() {
 				obj.TerminationMessagePath = TerminationMessagePathDefault
 			}
 		},
-		func(obj *RestartPolicy) {
-			if util.AllPtrFieldsNil(obj) {
-				obj.Always = &RestartPolicyAlways{}
-			}
-		},
 		func(obj *Service) {
 			if obj.Spec.Protocol == "" {
 				obj.Spec.Protocol = ProtocolTCP
@@ -69,6 +64,9 @@ func init() {
 			if obj.DNSPolicy == "" {
 				obj.DNSPolicy = DNSClusterFirst
 			}
+			if obj.RestartPolicy == "" {
+				obj.RestartPolicy = RestartPolicyAlways
+			}
 		},
 		func(obj *Probe) {
 			if obj.TimeoutSeconds == 0 {

diff --git a/pkg/api/v1beta3/defaults_test.go b/pkg/api/v1beta3/defaults_test.go
@@ -62,7 +62,7 @@ func TestSetDefaulPodSpec(t *testing.T) {
 		t.Errorf("Expected default dns policy :%s, got: %s", current.DNSClusterFirst, bp2.Spec.DNSPolicy)
 	}
 	policy := bp2.Spec.RestartPolicy
-	if policy.Never != nil || policy.OnFailure != nil || policy.Always == nil {
+	if policy != current.RestartPolicyAlways {
 		t.Errorf("Expected only policy.Always is set, got: %s", policy)
 	}
 	vsource := bp2.Spec.Volumes[0].VolumeSource

diff --git a/pkg/api/v1beta3/types.go b/pkg/api/v1beta3/types.go
@@ -522,22 +522,17 @@ type PodCondition struct {
 // PodInfo contains one entry for every container with available info.
 type PodInfo map[string]ContainerStatus
 
-type RestartPolicyAlways struct{}
+// RestartPolicy describes how the container should be restarted.
+// Only one of the following restart policies may be specified.
+// If none of the following policies is specified, the default one
+// is RestartPolicyAlways.
+type RestartPolicy string
 
-// TODO(dchen1107): Define what kinds of failures should restart.
-// TODO(dchen1107): Decide whether to support policy knobs, and, if so, which ones.
-type RestartPolicyOnFailure struct{}
-
-type RestartPolicyNever struct{}
-
-type RestartPolicy struct {
-	// Only one of the following restart policies may be specified.
-	// If none of the following policies is specified, the default one
-	// is RestartPolicyAlways.
-	Always    *RestartPolicyAlways    `json:"always,omitempty" description:"always restart the container after termination"`
-	OnFailure *RestartPolicyOnFailure `json:"onFailure,omitempty" description:"restart the container if it fails for any reason, but not if it succeeds (exit 0)"`
-	Never     *RestartPolicyNever     `json:"never,omitempty" description:"never restart the container"`
-}
+const (
+	RestartPolicyAlways    RestartPolicy = "Always"
+	RestartPolicyOnFailure RestartPolicy = "OnFailure"
+	RestartPolicyNever     RestartPolicy = "Never"
+)
 
 // DNSPolicy defines how a pod's DNS will be configured.
 type DNSPolicy string

diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go
@@ -600,20 +600,16 @@ func ValidateManifest(manifest *api.ContainerManifest) errs.ValidationErrorList
 }
 
 func validateRestartPolicy(restartPolicy *api.RestartPolicy) errs.ValidationErrorList {
-	numPolicies := 0
 	allErrors := errs.ValidationErrorList{}
-	if restartPolicy.Always != nil {
-		numPolicies++
-	}
-	if restartPolicy.OnFailure != nil {
-		numPolicies++
-	}
-	if restartPolicy.Never != nil {
-		numPolicies++
-	}
-	if numPolicies != 1 {
-		allErrors = append(allErrors, errs.NewFieldInvalid("", restartPolicy, "only 1 policy is allowed"))
+	switch *restartPolicy {
+	case api.RestartPolicyAlways, api.RestartPolicyOnFailure, api.RestartPolicyNever:
+		break
+	case "":
+		allErrors = append(allErrors, errs.NewFieldRequired("", *restartPolicy))
+	default:
+		allErrors = append(allErrors, errs.NewFieldNotSupported("", restartPolicy))
 	}
+
 	return allErrors
 }
 
@@ -786,10 +782,8 @@ func ValidateReplicationControllerSpec(spec *api.ReplicationControllerSpec) errs
 		}
 		allErrs = append(allErrs, ValidatePodTemplateSpec(spec.Template, spec.Replicas).Prefix("template")...)
 		// RestartPolicy has already been first-order validated as per ValidatePodTemplateSpec().
-		if spec.Template.Spec.RestartPolicy.Always == nil {
-			// TODO: should probably be Unsupported
-			// TODO: api.RestartPolicy should have a String() method for nicer printing
-			allErrs = append(allErrs, errs.NewFieldInvalid("template.restartPolicy", spec.Template.Spec.RestartPolicy, "must be Always"))
+		if spec.Template.Spec.RestartPolicy != api.RestartPolicyAlways {
+			allErrs = append(allErrs, errs.NewFieldNotSupported("template.restartPolicy", spec.Template.Spec.RestartPolicy))
 		}
 	}
 	return allErrs