From d13421e084eda5d898f312b41de473984e12d84d Mon Sep 17 00:00:00 2001 From: Kris Rousey Date: Mon, 15 Jun 2015 12:04:30 -0700 Subject: [PATCH] Removing ContainerManifest --- contrib/mesos/pkg/executor/executor_test.go | 2 +- pkg/api/conversion.go | 89 ------------------- pkg/api/deep_copy_generated.go | 74 --------------- pkg/api/latest/latest.go | 1 - pkg/api/register.go | 4 - pkg/api/serialization_test.go | 2 +- pkg/api/types.go | 37 -------- pkg/api/v1/types.go | 32 ------- pkg/api/v1beta3/types.go | 32 ------- pkg/api/validation/validation.go | 23 ----- pkg/controller/replication_controller_test.go | 5 -- pkg/kubelet/config/common.go | 5 -- pkg/kubelet/config/file_test.go | 37 +------- pkg/kubelet/config/http_test.go | 41 +++++---- pkg/kubelet/dockertools/docker_test.go | 2 +- 15 files changed, 26 insertions(+), 360 deletions(-) diff --git a/contrib/mesos/pkg/executor/executor_test.go b/contrib/mesos/pkg/executor/executor_test.go index 2678665616c..4f6dbea0f74 100644 --- a/contrib/mesos/pkg/executor/executor_test.go +++ b/contrib/mesos/pkg/executor/executor_test.go @@ -437,9 +437,9 @@ func TestExecutorStaticPods(t *testing.T) { assert.NoError(t, err) spod := `{ "apiVersion": "v1beta3", - "name": "%v", "kind": "Pod", "metadata": { + "name": "%v", "labels": { "name": "foo", "cluster": "bar" } }, "spec": { diff --git a/pkg/api/conversion.go b/pkg/api/conversion.go index 3bca8f3d763..918d0df7af3 100644 --- a/pkg/api/conversion.go +++ b/pkg/api/conversion.go @@ -81,94 +81,5 @@ func init() { *out = *in.Copy() return nil }, - // Convert ContainerManifest to Pod - func(in *ContainerManifest, out *Pod, s conversion.Scope) error { - out.Spec.Containers = in.Containers - out.Spec.Volumes = in.Volumes - out.Spec.RestartPolicy = in.RestartPolicy - out.Spec.DNSPolicy = in.DNSPolicy - out.Name = in.ID - out.UID = in.UUID - - if in.ID != "" { - out.SelfLink = "/api/v1beta1/pods/" + in.ID - } - - return nil - }, - func(in *Pod, out *ContainerManifest, s conversion.Scope) error { - out.Containers = in.Spec.Containers - out.Volumes = in.Spec.Volumes - out.RestartPolicy = in.Spec.RestartPolicy - out.DNSPolicy = in.Spec.DNSPolicy - out.Version = "v1beta2" - out.ID = in.Name - out.UUID = in.UID - return nil - }, - - // ContainerManifestList - func(in *ContainerManifestList, out *PodList, s conversion.Scope) error { - if err := s.Convert(&in.Items, &out.Items, 0); err != nil { - return err - } - for i := range out.Items { - item := &out.Items[i] - item.ResourceVersion = in.ResourceVersion - } - return nil - }, - func(in *PodList, out *ContainerManifestList, s conversion.Scope) error { - if err := s.Convert(&in.Items, &out.Items, 0); err != nil { - return err - } - out.ResourceVersion = in.ResourceVersion - return nil - }, - - // Conversion between Manifest and PodSpec - func(in *PodSpec, out *ContainerManifest, s conversion.Scope) error { - if err := s.Convert(&in.Volumes, &out.Volumes, 0); err != nil { - return err - } - if err := s.Convert(&in.Containers, &out.Containers, 0); err != nil { - return err - } - if err := s.Convert(&in.RestartPolicy, &out.RestartPolicy, 0); err != nil { - return err - } - if in.TerminationGracePeriodSeconds != nil { - out.TerminationGracePeriodSeconds = new(int64) - *out.TerminationGracePeriodSeconds = *in.TerminationGracePeriodSeconds - } - if in.ActiveDeadlineSeconds != nil { - out.ActiveDeadlineSeconds = new(int64) - *out.ActiveDeadlineSeconds = *in.ActiveDeadlineSeconds - } - out.DNSPolicy = in.DNSPolicy - out.Version = "v1beta2" - return nil - }, - func(in *ContainerManifest, out *PodSpec, s conversion.Scope) error { - if err := s.Convert(&in.Volumes, &out.Volumes, 0); err != nil { - return err - } - if err := s.Convert(&in.Containers, &out.Containers, 0); err != nil { - return err - } - if err := s.Convert(&in.RestartPolicy, &out.RestartPolicy, 0); err != nil { - return err - } - if in.TerminationGracePeriodSeconds != nil { - out.TerminationGracePeriodSeconds = new(int64) - *out.TerminationGracePeriodSeconds = *in.TerminationGracePeriodSeconds - } - if in.ActiveDeadlineSeconds != nil { - out.ActiveDeadlineSeconds = new(int64) - *out.ActiveDeadlineSeconds = *in.ActiveDeadlineSeconds - } - out.DNSPolicy = in.DNSPolicy - return nil - }, ) } diff --git a/pkg/api/deep_copy_generated.go b/pkg/api/deep_copy_generated.go index 1d45137a1de..a9ae6d7e1ab 100644 --- a/pkg/api/deep_copy_generated.go +++ b/pkg/api/deep_copy_generated.go @@ -207,78 +207,6 @@ func deepCopy_api_Container(in Container, out *Container, c *conversion.Cloner) return nil } -func deepCopy_api_ContainerManifest(in ContainerManifest, out *ContainerManifest, c *conversion.Cloner) error { - out.Version = in.Version - out.ID = in.ID - out.UUID = in.UUID - if in.Volumes != nil { - out.Volumes = make([]Volume, len(in.Volumes)) - for i := range in.Volumes { - if err := deepCopy_api_Volume(in.Volumes[i], &out.Volumes[i], c); err != nil { - return err - } - } - } else { - out.Volumes = nil - } - if in.Containers != nil { - out.Containers = make([]Container, len(in.Containers)) - for i := range in.Containers { - if err := deepCopy_api_Container(in.Containers[i], &out.Containers[i], c); err != nil { - return err - } - } - } else { - out.Containers = nil - } - out.RestartPolicy = in.RestartPolicy - if in.TerminationGracePeriodSeconds != nil { - out.TerminationGracePeriodSeconds = new(int64) - *out.TerminationGracePeriodSeconds = *in.TerminationGracePeriodSeconds - } else { - out.TerminationGracePeriodSeconds = nil - } - if in.ActiveDeadlineSeconds != nil { - out.ActiveDeadlineSeconds = new(int64) - *out.ActiveDeadlineSeconds = *in.ActiveDeadlineSeconds - } else { - out.ActiveDeadlineSeconds = nil - } - out.DNSPolicy = in.DNSPolicy - out.HostNetwork = in.HostNetwork - if in.ImagePullSecrets != nil { - out.ImagePullSecrets = make([]LocalObjectReference, len(in.ImagePullSecrets)) - for i := range in.ImagePullSecrets { - if err := deepCopy_api_LocalObjectReference(in.ImagePullSecrets[i], &out.ImagePullSecrets[i], c); err != nil { - return err - } - } - } else { - out.ImagePullSecrets = nil - } - return nil -} - -func deepCopy_api_ContainerManifestList(in ContainerManifestList, out *ContainerManifestList, c *conversion.Cloner) error { - if err := deepCopy_api_TypeMeta(in.TypeMeta, &out.TypeMeta, c); err != nil { - return err - } - if err := deepCopy_api_ListMeta(in.ListMeta, &out.ListMeta, c); err != nil { - return err - } - if in.Items != nil { - out.Items = make([]ContainerManifest, len(in.Items)) - for i := range in.Items { - if err := deepCopy_api_ContainerManifest(in.Items[i], &out.Items[i], c); err != nil { - return err - } - } - } else { - out.Items = nil - } - return nil -} - func deepCopy_api_ContainerPort(in ContainerPort, out *ContainerPort, c *conversion.Cloner) error { out.Name = in.Name out.HostPort = in.HostPort @@ -2151,8 +2079,6 @@ func init() { deepCopy_api_ComponentStatus, deepCopy_api_ComponentStatusList, deepCopy_api_Container, - deepCopy_api_ContainerManifest, - deepCopy_api_ContainerManifestList, deepCopy_api_ContainerPort, deepCopy_api_ContainerState, deepCopy_api_ContainerStateRunning, diff --git a/pkg/api/latest/latest.go b/pkg/api/latest/latest.go index 02ef3f2cd0b..c4ff2f5f69e 100644 --- a/pkg/api/latest/latest.go +++ b/pkg/api/latest/latest.go @@ -103,7 +103,6 @@ func init() { "ListOptions", "DeleteOptions", "Status", - "ContainerManifest", "PodLogOptions", "PodExecOptions", "PodProxyOptions") diff --git a/pkg/api/register.go b/pkg/api/register.go index ca879ffd32f..e0ecfb38e18 100644 --- a/pkg/api/register.go +++ b/pkg/api/register.go @@ -42,8 +42,6 @@ func init() { &Binding{}, &Event{}, &EventList{}, - &ContainerManifest{}, - &ContainerManifestList{}, &List{}, &LimitRange{}, &LimitRangeList{}, @@ -91,8 +89,6 @@ func (*Binding) IsAnAPIObject() {} func (*Status) IsAnAPIObject() {} func (*Event) IsAnAPIObject() {} func (*EventList) IsAnAPIObject() {} -func (*ContainerManifest) IsAnAPIObject() {} -func (*ContainerManifestList) IsAnAPIObject() {} func (*List) IsAnAPIObject() {} func (*LimitRange) IsAnAPIObject() {} func (*LimitRangeList) IsAnAPIObject() {} diff --git a/pkg/api/serialization_test.go b/pkg/api/serialization_test.go index c206e94e78d..5b41239d80e 100644 --- a/pkg/api/serialization_test.go +++ b/pkg/api/serialization_test.go @@ -125,7 +125,7 @@ func TestList(t *testing.T) { roundTripSame(t, item) } -var nonRoundTrippableTypes = util.NewStringSet("ContainerManifest", "ContainerManifestList") +var nonRoundTrippableTypes = util.NewStringSet() var nonInternalRoundTrippableTypes = util.NewStringSet("List", "ListOptions", "PodExecOptions") var nonRoundTrippableTypesByVersion = map[string][]string{} diff --git a/pkg/api/types.go b/pkg/api/types.go index 81e4c897217..6011fe33389 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -1814,43 +1814,6 @@ type EventList struct { Items []Event `json:"items"` } -// ContainerManifest corresponds to the Container Manifest format, documented at: -// https://developers.google.com/compute/docs/containers/container_vms#container_manifest -// This is used as the representation of Kubernetes workloads. -// DEPRECATED: Replaced with Pod -type ContainerManifest struct { - // Required: This must be a supported version string, such as "v1beta1". - Version string `json:"version"` - // Required: This must be a DNS_SUBDOMAIN. - // TODO: ID on Manifest is deprecated and will be removed in the future. - ID string `json:"id"` - // TODO: UUID on Manifest is deprecated in the future once we are done - // with the API refactoring. It is required for now to determine the instance - // of a Pod. - UUID types.UID `json:"uuid,omitempty"` - Volumes []Volume `json:"volumes"` - Containers []Container `json:"containers"` - RestartPolicy RestartPolicy `json:"restartPolicy,omitempty"` - TerminationGracePeriodSeconds *int64 `json:"terminationGracePeriodSeconds,omitempty"` - ActiveDeadlineSeconds *int64 `json:"activeDeadlineSeconds,omitempty"` - // Required: Set DNS policy. - DNSPolicy DNSPolicy `json:"dnsPolicy"` - HostNetwork bool `json:"hostNetwork,omitempty"` - // ImagePullSecrets is an optional list of references to secrets in the same namespace to use for pulling any of the images used by this PodSpec. - // If specified, these secrets will be passed to individual puller implementations for them to use. For example, - // in the case of docker, only DockerConfig type secrets are honored. - ImagePullSecrets []LocalObjectReference `json:"imagePullSecrets,omitempty" description:"list of references to secrets in the same namespace available for pulling the container images"` -} - -// ContainerManifestList is used to communicate container manifests to kubelet. -// DEPRECATED: Replaced with Pods -type ContainerManifestList struct { - TypeMeta `json:",inline"` - ListMeta `json:"metadata,omitempty"` - - Items []ContainerManifest `json:"items"` -} - // List holds a list of objects, which may not be known by the server. type List struct { TypeMeta `json:",inline"` diff --git a/pkg/api/v1/types.go b/pkg/api/v1/types.go index e39456d7054..421910f6dd8 100644 --- a/pkg/api/v1/types.go +++ b/pkg/api/v1/types.go @@ -152,38 +152,6 @@ const ( NamespaceAll string = "" ) -// -//// ContainerManifest corresponds to the Container Manifest format, documented at: -//// https://developers.google.com/compute/docs/containers/container_vms#container_manifest -//// This is used as the representation of Kubernetes workloads. -//// DEPRECATED: Exists to allow backwards compatible storage for clients accessing etcd -//// directly. -//type ContainerManifest struct { -// // Required: This must be a supported version string, such as "v1beta1". -// Version string `json:"version"` -// // Required: This must be a DNS_SUBDOMAIN. -// // TODO: ID on Manifest is deprecated and will be removed in the future. -// ID string `json:"id"` -// // TODO: UUID on Manifest is deprecated in the future once we are done -// // with the API refactoring. It is required for now to determine the instance -// // of a Pod. -// UUID types.UID `json:"uuid,omitempty"` -// Volumes []Volume `json:"volumes"` -// Containers []Container `json:"containers"` -// RestartPolicy RestartPolicy `json:"restartPolicy,omitempty"` -//} -// -//// ContainerManifestList is used to communicate container manifests to kubelet. -//// DEPRECATED: Exists to allow backwards compatible storage for clients accessing etcd -//// directly. -//type ContainerManifestList struct { -// TypeMeta `json:",inline"` -// // ID is the legacy field representing Name -// ID string `json:"id,omitempty"` -// -// Items []ContainerManifest `json:"items,omitempty"` -//} - // Volume represents a named volume in a pod that may be accessed by any containers in the pod. type Volume struct { // Required: This must be a DNS_LABEL. Each volume in a pod must have diff --git a/pkg/api/v1beta3/types.go b/pkg/api/v1beta3/types.go index 63323dad7df..d37c14d1f2c 100644 --- a/pkg/api/v1beta3/types.go +++ b/pkg/api/v1beta3/types.go @@ -152,38 +152,6 @@ const ( NamespaceAll string = "" ) -// -//// ContainerManifest corresponds to the Container Manifest format, documented at: -//// https://developers.google.com/compute/docs/containers/container_vms#container_manifest -//// This is used as the representation of Kubernetes workloads. -//// DEPRECATED: Exists to allow backwards compatible storage for clients accessing etcd -//// directly. -//type ContainerManifest struct { -// // Required: This must be a supported version string, such as "v1beta1". -// Version string `json:"version"` -// // Required: This must be a DNS_SUBDOMAIN. -// // TODO: ID on Manifest is deprecated and will be removed in the future. -// ID string `json:"id"` -// // TODO: UUID on Manifest is deprecated in the future once we are done -// // with the API refactoring. It is required for now to determine the instance -// // of a Pod. -// UUID types.UID `json:"uuid,omitempty"` -// Volumes []Volume `json:"volumes"` -// Containers []Container `json:"containers"` -// RestartPolicy RestartPolicy `json:"restartPolicy,omitempty"` -//} -// -//// ContainerManifestList is used to communicate container manifests to kubelet. -//// DEPRECATED: Exists to allow backwards compatible storage for clients accessing etcd -//// directly. -//type ContainerManifestList struct { -// TypeMeta `json:",inline"` -// // ID is the legacy field representing Name -// ID string `json:"id,omitempty"` -// -// Items []ContainerManifest `json:"items,omitempty"` -//} - // Volume represents a named volume in a pod that may be accessed by any containers in the pod. type Volume struct { // Required: This must be a DNS_LABEL. Each volume in a pod must have diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index 6278597f4ba..a32dee4bd33 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -863,29 +863,6 @@ func validateContainers(containers []api.Container, volumes util.StringSet) errs return allErrs } -var supportedManifestVersions = util.NewStringSet("v1beta1", "v1beta2") - -// ValidateManifest tests that the specified ContainerManifest has valid data. -// This includes checking formatting and uniqueness. It also canonicalizes the -// structure by setting default values and implementing any backwards-compatibility -// tricks. -// TODO: replaced by ValidatePodSpec -func ValidateManifest(manifest *api.ContainerManifest) errs.ValidationErrorList { - allErrs := errs.ValidationErrorList{} - - if len(manifest.Version) == 0 { - allErrs = append(allErrs, errs.NewFieldRequired("version")) - } else if !supportedManifestVersions.Has(strings.ToLower(manifest.Version)) { - allErrs = append(allErrs, errs.NewFieldNotSupported("version", manifest.Version)) - } - allVolumes, vErrs := validateVolumes(manifest.Volumes) - allErrs = append(allErrs, vErrs.Prefix("volumes")...) - allErrs = append(allErrs, validateContainers(manifest.Containers, allVolumes).Prefix("containers")...) - allErrs = append(allErrs, validateRestartPolicy(&manifest.RestartPolicy).Prefix("restartPolicy")...) - allErrs = append(allErrs, validateDNSPolicy(&manifest.DNSPolicy).Prefix("dnsPolicy")...) - return allErrs -} - func validateRestartPolicy(restartPolicy *api.RestartPolicy) errs.ValidationErrorList { allErrors := errs.ValidationErrorList{} switch *restartPolicy { diff --git a/pkg/controller/replication_controller_test.go b/pkg/controller/replication_controller_test.go index 88ec1c953b7..93561f554c2 100644 --- a/pkg/controller/replication_controller_test.go +++ b/pkg/controller/replication_controller_test.go @@ -316,11 +316,6 @@ func TestCreateReplica(t *testing.T) { // Make sure createReplica sends a POST to the apiserver with a pod from the controllers pod template podControl.createReplica(ns, controllerSpec) - manifest := api.ContainerManifest{} - if err := api.Scheme.Convert(&controllerSpec.Spec.Template.Spec, &manifest); err != nil { - t.Fatalf("unexpected error: %v", err) - } - expectedPod := api.Pod{ ObjectMeta: api.ObjectMeta{ Labels: controllerSpec.Spec.Template.Labels, diff --git a/pkg/kubelet/config/common.go b/pkg/kubelet/config/common.go index d5a4bd42799..c7178ce1657 100644 --- a/pkg/kubelet/config/common.go +++ b/pkg/kubelet/config/common.go @@ -52,11 +52,6 @@ func applyDefaults(pod *api.Pod, source string, isFile bool, nodeName string) er glog.V(5).Infof("Generated UID %q pod %q from %s", pod.UID, pod.Name, source) } - // This is required for backward compatibility, and should be removed once we - // completely deprecate ContainerManifest. - if len(pod.Name) == 0 { - pod.Name = string(pod.UID) - } pod.Name = generatePodName(pod.Name, nodeName) glog.V(5).Infof("Generated Name %q for UID %q from URL %s", pod.Name, pod.UID, source) diff --git a/pkg/kubelet/config/file_test.go b/pkg/kubelet/config/file_test.go index c7b115665d4..9f9d423dfc9 100644 --- a/pkg/kubelet/config/file_test.go +++ b/pkg/kubelet/config/file_test.go @@ -110,41 +110,6 @@ func TestReadPodsFromFile(t *testing.T) { }, }), }, - { - desc: "Pod without ID", - pod: &api.Pod{ - TypeMeta: api.TypeMeta{ - Kind: "Pod", - APIVersion: "", - }, - ObjectMeta: api.ObjectMeta{ - // No name - UID: "12345", - }, - Spec: api.PodSpec{ - Containers: []api.Container{{Name: "image", Image: "test/image", SecurityContext: securitycontext.ValidSecurityContextWithContainerDefaults()}}, - }, - }, - expected: CreatePodUpdate(kubelet.SET, kubelet.FileSource, &api.Pod{ - ObjectMeta: api.ObjectMeta{ - Name: "12345-" + hostname, - UID: "12345", - Namespace: kubelet.NamespaceDefault, - SelfLink: getSelfLink("12345-"+hostname, kubelet.NamespaceDefault), - }, - Spec: api.PodSpec{ - NodeName: hostname, - RestartPolicy: api.RestartPolicyAlways, - DNSPolicy: api.DNSClusterFirst, - Containers: []api.Container{{ - Name: "image", - Image: "test/image", - TerminationMessagePath: "/dev/termination-log", - ImagePullPolicy: "IfNotPresent", - SecurityContext: securitycontext.ValidSecurityContextWithContainerDefaults()}}, - }, - }), - }, } for _, testCase := range testCases { @@ -152,7 +117,7 @@ func TestReadPodsFromFile(t *testing.T) { var versionedPod runtime.Object err := testapi.Converter().Convert(&testCase.pod, &versionedPod) if err != nil { - t.Fatalf("error in versioning the pod: %s", testCase.desc, err) + t.Fatalf("%s: error in versioning the pod: %v", testCase.desc, err) } fileContents, err := testapi.Codec().Encode(versionedPod) if err != nil { diff --git a/pkg/kubelet/config/http_test.go b/pkg/kubelet/config/http_test.go index 26309fdf279..ab3487993bf 100644 --- a/pkg/kubelet/config/http_test.go +++ b/pkg/kubelet/config/http_test.go @@ -50,55 +50,58 @@ func TestExtractFromHttpBadness(t *testing.T) { expectEmptyChannel(t, ch) } -func TestExtractInvalidManifest(t *testing.T) { +func TestExtractInvalidPods(t *testing.T) { var testCases = []struct { - desc string - manifests interface{} + desc string + pod *api.Pod }{ { - desc: "No version", - manifests: []api.ContainerManifest{{Version: ""}}, + desc: "No version", + pod: &api.Pod{TypeMeta: api.TypeMeta{APIVersion: ""}}, }, { - desc: "Invalid version", - manifests: []api.ContainerManifest{{Version: "v1betta2"}}, + desc: "Invalid version", + pod: &api.Pod{TypeMeta: api.TypeMeta{APIVersion: "v1betta2"}}, }, { desc: "Invalid volume name", - manifests: []api.ContainerManifest{ - {Version: testapi.Version(), Volumes: []api.Volume{{Name: "_INVALID_"}}}, + pod: &api.Pod{ + TypeMeta: api.TypeMeta{APIVersion: testapi.Version()}, + Spec: api.PodSpec{ + Volumes: []api.Volume{{Name: "_INVALID_"}}, + }, }, }, { desc: "Duplicate volume names", - manifests: []api.ContainerManifest{ - { - Version: testapi.Version(), + pod: &api.Pod{ + TypeMeta: api.TypeMeta{APIVersion: testapi.Version()}, + Spec: api.PodSpec{ Volumes: []api.Volume{{Name: "repeated"}, {Name: "repeated"}}, }, }, }, { desc: "Unspecified container name", - manifests: []api.ContainerManifest{ - { - Version: testapi.Version(), + pod: &api.Pod{ + TypeMeta: api.TypeMeta{APIVersion: testapi.Version()}, + Spec: api.PodSpec{ Containers: []api.Container{{Name: ""}}, }, }, }, { desc: "Invalid container name", - manifests: []api.ContainerManifest{ - { - Version: testapi.Version(), + pod: &api.Pod{ + TypeMeta: api.TypeMeta{APIVersion: testapi.Version()}, + Spec: api.PodSpec{ Containers: []api.Container{{Name: "_INVALID_"}}, }, }, }, } for _, testCase := range testCases { - data, err := json.Marshal(testCase.manifests) + data, err := json.Marshal(testCase.pod) if err != nil { t.Fatalf("%s: Some weird json problem: %v", testCase.desc, err) } diff --git a/pkg/kubelet/dockertools/docker_test.go b/pkg/kubelet/dockertools/docker_test.go index 97c260ff942..98e23a1538e 100644 --- a/pkg/kubelet/dockertools/docker_test.go +++ b/pkg/kubelet/dockertools/docker_test.go @@ -109,7 +109,7 @@ func verifyPackUnpack(t *testing.T, podNamespace, podUID, podName, containerName } } -func TestContainerManifestNaming(t *testing.T) { +func TestContainerNaming(t *testing.T) { podUID := "12345678" verifyPackUnpack(t, "file", podUID, "name", "container") verifyPackUnpack(t, "file", podUID, "name-with-dashes", "container")