Merge pull request #924 from thockin/cleanups2

Validation return types cleanup
This commit is contained in:
Daniel Smith 2014-08-18 09:38:07 -07:00
commit b00afd7f3b
2 changed files with 49 additions and 54 deletions

View File

@ -90,11 +90,6 @@ func (list errorListInternal) Error() string {
return strings.Join(sl, "; ") return strings.Join(sl, "; ")
} }
// Deprecated, will be removed soon.
func (list *ErrorList) Append(errs ...error) {
*list = append(*list, errs...)
}
// ToError converts an ErrorList into a "normal" error, or nil if the list is empty. // ToError converts an ErrorList into a "normal" error, or nil if the list is empty.
func (list ErrorList) ToError() error { func (list ErrorList) ToError() error {
if len(list) == 0 { if len(list) == 0 {

View File

@ -37,14 +37,14 @@ func validateVolumes(volumes []Volume) (util.StringSet, errs.ErrorList) {
el = validateSource(vol.Source) el = validateSource(vol.Source)
} }
if !util.IsDNSLabel(vol.Name) { if !util.IsDNSLabel(vol.Name) {
el.Append(errs.NewInvalid("Volume.Name", vol.Name)) el = append(el, errs.NewInvalid("Volume.Name", vol.Name))
} else if allNames.Has(vol.Name) { } else if allNames.Has(vol.Name) {
el.Append(errs.NewDuplicate("Volume.Name", vol.Name)) el = append(el, errs.NewDuplicate("Volume.Name", vol.Name))
} }
if len(el) == 0 { if len(el) == 0 {
allNames.Insert(vol.Name) allNames.Insert(vol.Name)
} else { } else {
allErrs.Append(el...) allErrs = append(allErrs, el...)
} }
} }
return allNames, allErrs return allNames, allErrs
@ -55,14 +55,14 @@ func validateSource(source *VolumeSource) errs.ErrorList {
allErrs := errs.ErrorList{} allErrs := errs.ErrorList{}
if source.HostDirectory != nil { if source.HostDirectory != nil {
numVolumes++ numVolumes++
allErrs.Append(validateHostDir(source.HostDirectory)...) allErrs = append(allErrs, validateHostDir(source.HostDirectory)...)
} }
if source.EmptyDirectory != nil { if source.EmptyDirectory != nil {
numVolumes++ numVolumes++
//EmptyDirs have nothing to validate //EmptyDirs have nothing to validate
} }
if numVolumes != 1 { if numVolumes != 1 {
allErrs.Append(errs.NewInvalid("Volume.Source", source)) allErrs = append(allErrs, errs.NewInvalid("Volume.Source", source))
} }
return allErrs return allErrs
} }
@ -70,7 +70,7 @@ func validateSource(source *VolumeSource) errs.ErrorList {
func validateHostDir(hostDir *HostDirectory) errs.ErrorList { func validateHostDir(hostDir *HostDirectory) errs.ErrorList {
allErrs := errs.ErrorList{} allErrs := errs.ErrorList{}
if hostDir.Path == "" { if hostDir.Path == "" {
allErrs.Append(errs.NewNotFound("HostDir.Path", hostDir.Path)) allErrs = append(allErrs, errs.NewNotFound("HostDir.Path", hostDir.Path))
} }
return allErrs return allErrs
} }
@ -85,25 +85,25 @@ func validatePorts(ports []Port) errs.ErrorList {
port := &ports[i] // so we can set default values port := &ports[i] // so we can set default values
if len(port.Name) > 0 { if len(port.Name) > 0 {
if len(port.Name) > 63 || !util.IsDNSLabel(port.Name) { if len(port.Name) > 63 || !util.IsDNSLabel(port.Name) {
allErrs.Append(errs.NewInvalid("Port.Name", port.Name)) allErrs = append(allErrs, errs.NewInvalid("Port.Name", port.Name))
} else if allNames.Has(port.Name) { } else if allNames.Has(port.Name) {
allErrs.Append(errs.NewDuplicate("Port.name", port.Name)) allErrs = append(allErrs, errs.NewDuplicate("Port.name", port.Name))
} else { } else {
allNames.Insert(port.Name) allNames.Insert(port.Name)
} }
} }
if !util.IsValidPortNum(port.ContainerPort) { if !util.IsValidPortNum(port.ContainerPort) {
allErrs.Append(errs.NewInvalid("Port.ContainerPort", port.ContainerPort)) allErrs = append(allErrs, errs.NewInvalid("Port.ContainerPort", port.ContainerPort))
} }
if port.HostPort == 0 { if port.HostPort == 0 {
port.HostPort = port.ContainerPort port.HostPort = port.ContainerPort
} else if !util.IsValidPortNum(port.HostPort) { } else if !util.IsValidPortNum(port.HostPort) {
allErrs.Append(errs.NewInvalid("Port.HostPort", port.HostPort)) allErrs = append(allErrs, errs.NewInvalid("Port.HostPort", port.HostPort))
} }
if len(port.Protocol) == 0 { if len(port.Protocol) == 0 {
port.Protocol = "TCP" port.Protocol = "TCP"
} else if !supportedPortProtocols.Has(strings.ToUpper(port.Protocol)) { } else if !supportedPortProtocols.Has(strings.ToUpper(port.Protocol)) {
allErrs.Append(errs.NewNotSupported("Port.Protocol", port.Protocol)) allErrs = append(allErrs, errs.NewNotSupported("Port.Protocol", port.Protocol))
} }
} }
return allErrs return allErrs
@ -115,10 +115,10 @@ func validateEnv(vars []EnvVar) errs.ErrorList {
for i := range vars { for i := range vars {
ev := &vars[i] // so we can set default values ev := &vars[i] // so we can set default values
if len(ev.Name) == 0 { if len(ev.Name) == 0 {
allErrs.Append(errs.NewInvalid("EnvVar.Name", ev.Name)) allErrs = append(allErrs, errs.NewInvalid("EnvVar.Name", ev.Name))
} }
if !util.IsCIdentifier(ev.Name) { if !util.IsCIdentifier(ev.Name) {
allErrs.Append(errs.NewInvalid("EnvVar.Name", ev.Name)) allErrs = append(allErrs, errs.NewInvalid("EnvVar.Name", ev.Name))
} }
} }
return allErrs return allErrs
@ -130,14 +130,14 @@ func validateVolumeMounts(mounts []VolumeMount, volumes util.StringSet) errs.Err
for i := range mounts { for i := range mounts {
mnt := &mounts[i] // so we can set default values mnt := &mounts[i] // so we can set default values
if len(mnt.Name) == 0 { if len(mnt.Name) == 0 {
allErrs.Append(errs.NewInvalid("VolumeMount.Name", mnt.Name)) allErrs = append(allErrs, errs.NewInvalid("VolumeMount.Name", mnt.Name))
} else if !volumes.Has(mnt.Name) { } else if !volumes.Has(mnt.Name) {
allErrs.Append(errs.NewNotFound("VolumeMount.Name", mnt.Name)) allErrs = append(allErrs, errs.NewNotFound("VolumeMount.Name", mnt.Name))
} }
if len(mnt.MountPath) == 0 { if len(mnt.MountPath) == 0 {
// Backwards compat. // Backwards compat.
if len(mnt.Path) == 0 { if len(mnt.Path) == 0 {
allErrs.Append(errs.NewInvalid("VolumeMount.MountPath", mnt.MountPath)) allErrs = append(allErrs, errs.NewInvalid("VolumeMount.MountPath", mnt.MountPath))
} else { } else {
glog.Warning("DEPRECATED: VolumeMount.Path has been replaced by VolumeMount.MountPath") glog.Warning("DEPRECATED: VolumeMount.Path has been replaced by VolumeMount.MountPath")
mnt.MountPath = mnt.Path mnt.MountPath = mnt.Path
@ -161,7 +161,7 @@ func AccumulateUniquePorts(containers []Container, accumulator map[int]bool, ext
for pi := range ctr.Ports { for pi := range ctr.Ports {
port := extract(&ctr.Ports[pi]) port := extract(&ctr.Ports[pi])
if accumulator[port] { if accumulator[port] {
allErrs.Append(errs.NewDuplicate("Port", port)) allErrs = append(allErrs, errs.NewDuplicate("Port", port))
} else { } else {
accumulator[port] = true accumulator[port] = true
} }
@ -183,25 +183,25 @@ func validateContainers(containers []Container, volumes util.StringSet) errs.Err
for i := range containers { for i := range containers {
ctr := &containers[i] // so we can set default values ctr := &containers[i] // so we can set default values
if !util.IsDNSLabel(ctr.Name) { if !util.IsDNSLabel(ctr.Name) {
allErrs.Append(errs.NewInvalid("Container.Name", ctr.Name)) allErrs = append(allErrs, errs.NewInvalid("Container.Name", ctr.Name))
} else if allNames.Has(ctr.Name) { } else if allNames.Has(ctr.Name) {
allErrs.Append(errs.NewDuplicate("Container.Name", ctr.Name)) allErrs = append(allErrs, errs.NewDuplicate("Container.Name", ctr.Name))
} else { } else {
allNames.Insert(ctr.Name) allNames.Insert(ctr.Name)
} }
if len(ctr.Image) == 0 { if len(ctr.Image) == 0 {
allErrs.Append(errs.NewInvalid("Container.Image", ctr.Name)) allErrs = append(allErrs, errs.NewInvalid("Container.Image", ctr.Name))
} }
allErrs.Append(validatePorts(ctr.Ports)...) allErrs = append(allErrs, validatePorts(ctr.Ports)...)
allErrs.Append(validateEnv(ctr.Env)...) allErrs = append(allErrs, validateEnv(ctr.Env)...)
allErrs.Append(validateVolumeMounts(ctr.VolumeMounts, volumes)...) allErrs = append(allErrs, validateVolumeMounts(ctr.VolumeMounts, volumes)...)
} }
// Check for colliding ports across all containers. // Check for colliding ports across all containers.
// TODO(thockin): This really is dependent on the network config of the host (IP per pod?) // TODO(thockin): This really is dependent on the network config of the host (IP per pod?)
// and the config of the new manifest. But we have not specced that out yet, so we'll just // and the config of the new manifest. But we have not specced that out yet, so we'll just
// make some assumptions for now. As of now, pods share a network namespace, which means that // make some assumptions for now. As of now, pods share a network namespace, which means that
// every Port.HostPort across the whole pod must be unique. // every Port.HostPort across the whole pod must be unique.
allErrs.Append(checkHostPortConflicts(containers)...) allErrs = append(allErrs, checkHostPortConflicts(containers)...)
return allErrs return allErrs
} }
@ -212,71 +212,71 @@ var supportedManifestVersions = util.NewStringSet("v1beta1", "v1beta2")
// This includes checking formatting and uniqueness. It also canonicalizes the // This includes checking formatting and uniqueness. It also canonicalizes the
// structure by setting default values and implementing any backwards-compatibility // structure by setting default values and implementing any backwards-compatibility
// tricks. // tricks.
func ValidateManifest(manifest *ContainerManifest) []error { func ValidateManifest(manifest *ContainerManifest) errs.ErrorList {
allErrs := errs.ErrorList{} allErrs := errs.ErrorList{}
if len(manifest.Version) == 0 { if len(manifest.Version) == 0 {
allErrs.Append(errs.NewInvalid("ContainerManifest.Version", manifest.Version)) allErrs = append(allErrs, errs.NewInvalid("ContainerManifest.Version", manifest.Version))
} else if !supportedManifestVersions.Has(strings.ToLower(manifest.Version)) { } else if !supportedManifestVersions.Has(strings.ToLower(manifest.Version)) {
allErrs.Append(errs.NewNotSupported("ContainerManifest.Version", manifest.Version)) allErrs = append(allErrs, errs.NewNotSupported("ContainerManifest.Version", manifest.Version))
} }
allVolumes, errs := validateVolumes(manifest.Volumes) allVolumes, errs := validateVolumes(manifest.Volumes)
if len(errs) != 0 { if len(errs) != 0 {
allErrs.Append(errs...) allErrs = append(allErrs, errs...)
} }
allErrs.Append(validateContainers(manifest.Containers, allVolumes)...) allErrs = append(allErrs, validateContainers(manifest.Containers, allVolumes)...)
return []error(allErrs) return allErrs
} }
func ValidatePodState(podState *PodState) []error { func ValidatePodState(podState *PodState) errs.ErrorList {
allErrs := errs.ErrorList(ValidateManifest(&podState.Manifest)) allErrs := errs.ErrorList(ValidateManifest(&podState.Manifest))
if podState.RestartPolicy.Type == "" { if podState.RestartPolicy.Type == "" {
podState.RestartPolicy.Type = RestartAlways podState.RestartPolicy.Type = RestartAlways
} else if podState.RestartPolicy.Type != RestartAlways && } else if podState.RestartPolicy.Type != RestartAlways &&
podState.RestartPolicy.Type != RestartOnFailure && podState.RestartPolicy.Type != RestartOnFailure &&
podState.RestartPolicy.Type != RestartNever { podState.RestartPolicy.Type != RestartNever {
allErrs.Append(errs.NewNotSupported("PodState.RestartPolicy.Type", podState.RestartPolicy.Type)) allErrs = append(allErrs, errs.NewNotSupported("PodState.RestartPolicy.Type", podState.RestartPolicy.Type))
} }
return []error(allErrs) return allErrs
} }
// Pod tests if required fields in the pod are set. // Pod tests if required fields in the pod are set.
func ValidatePod(pod *Pod) []error { func ValidatePod(pod *Pod) errs.ErrorList {
allErrs := errs.ErrorList{} allErrs := errs.ErrorList{}
if pod.ID == "" { if pod.ID == "" {
allErrs.Append(errs.NewInvalid("Pod.ID", pod.ID)) allErrs = append(allErrs, errs.NewInvalid("Pod.ID", pod.ID))
} }
allErrs.Append(ValidatePodState(&pod.DesiredState)...) allErrs = append(allErrs, ValidatePodState(&pod.DesiredState)...)
return []error(allErrs) return allErrs
} }
// ValidateService tests if required fields in the service are set. // ValidateService tests if required fields in the service are set.
func ValidateService(service *Service) []error { func ValidateService(service *Service) errs.ErrorList {
allErrs := errs.ErrorList{} allErrs := errs.ErrorList{}
if service.ID == "" { if service.ID == "" {
allErrs.Append(errs.NewInvalid("Service.ID", service.ID)) allErrs = append(allErrs, errs.NewInvalid("Service.ID", service.ID))
} else if !util.IsDNS952Label(service.ID) { } else if !util.IsDNS952Label(service.ID) {
allErrs.Append(errs.NewInvalid("Service.ID", service.ID)) allErrs = append(allErrs, errs.NewInvalid("Service.ID", service.ID))
} }
if labels.Set(service.Selector).AsSelector().Empty() { if labels.Set(service.Selector).AsSelector().Empty() {
allErrs.Append(errs.NewInvalid("Service.Selector", service.Selector)) allErrs = append(allErrs, errs.NewInvalid("Service.Selector", service.Selector))
} }
return []error(allErrs) return allErrs
} }
// ValidateReplicationController tests if required fields in the replication controller are set. // ValidateReplicationController tests if required fields in the replication controller are set.
func ValidateReplicationController(controller *ReplicationController) []error { func ValidateReplicationController(controller *ReplicationController) errs.ErrorList {
el := []error{} allErrs := errs.ErrorList{}
if controller.ID == "" { if controller.ID == "" {
el = append(el, errs.NewInvalid("ReplicationController.ID", controller.ID)) allErrs = append(allErrs, errs.NewInvalid("ReplicationController.ID", controller.ID))
} }
if labels.Set(controller.DesiredState.ReplicaSelector).AsSelector().Empty() { if labels.Set(controller.DesiredState.ReplicaSelector).AsSelector().Empty() {
el = append(el, errs.NewInvalid("ReplicationController.ReplicaSelector", controller.DesiredState.ReplicaSelector)) allErrs = append(allErrs, errs.NewInvalid("ReplicationController.ReplicaSelector", controller.DesiredState.ReplicaSelector))
} }
if controller.DesiredState.Replicas < 0 { if controller.DesiredState.Replicas < 0 {
el = append(el, errs.NewInvalid("ReplicationController.Replicas", controller.DesiredState.Replicas)) allErrs = append(allErrs, errs.NewInvalid("ReplicationController.Replicas", controller.DesiredState.Replicas))
} }
el = append(el, ValidateManifest(&controller.DesiredState.PodTemplate.DesiredState.Manifest)...) allErrs = append(allErrs, ValidateManifest(&controller.DesiredState.PodTemplate.DesiredState.Manifest)...)
return el return allErrs
} }