Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose versioned query parameters and make watch an operation on List #5763

Merged
merged 3 commits into from
Mar 27, 2015
Merged

Expose versioned query parameters and make watch an operation on List #5763

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1618c39
Convert List query parameters via object conversion
smarterclayton Mar 22, 2015
eb0eff6
Move watch to being a resthandler resource and expose it on LIST
smarterclayton Mar 24, 2015
870da68
Add more extensive tests to apiserver for variations in version
smarterclayton Mar 25, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions hack/lib/util.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,16 @@ kube::util::wait_for_url() {
return 1
}

# Create a temp dir that'll be deleted at the end of this bash session.
#
# Vars set:
# KUBE_TEMP
kube::util::ensure-temp-dir() {
if [[ -z ${KUBE_TEMP-} ]]; then
KUBE_TEMP=$(mktemp -d -t kubernetes.XXXXXX)
fi
}

# This figures out the host platform without relying on golang. We need this as
# we don't want a golang install to be a prerequisite to building yet we need
# this info to figure out where the final binaries are placed.
Expand Down
18 changes: 18 additions & 0 deletions hack/test-cmd.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,14 @@ function cleanup()
[[ -n ${PROXY_PID-} ]] && kill ${PROXY_PID} 1>&2 2>/dev/null

kube::etcd::cleanup
rm -rf "${KUBE_TEMP}"

kube::log::status "Clean up complete"
}

trap cleanup EXIT SIGINT

kube::util::ensure-temp-dir
kube::etcd::start

ETCD_HOST=${ETCD_HOST:-127.0.0.1}
Expand Down Expand Up @@ -533,6 +535,7 @@ __EOF__

kube::test::describe_object_assert nodes "127.0.0.1" "Name:" "Labels:" "CreationTimestamp:" "Conditions:" "Addresses:" "Capacity:" "Pods:"


###########
# Minions #
###########
Expand All @@ -548,13 +551,28 @@ __EOF__
kube::test::describe_object_assert minions "127.0.0.1" "Name:" "Conditions:" "Addresses:" "Capacity:" "Pods:"
fi


#####################
# Retrieve multiple #
#####################

kube::log::status "Testing kubectl(${version}:multiget)"
kube::test::get_object_assert 'nodes/127.0.0.1 service/kubernetes' "{{range.items}}{{.$id_field}}:{{end}}" '127.0.0.1:kubernetes:'


###########
# Swagger #
###########

if [[ -n "${version}" ]]; then
# Verify schema
file="${KUBE_TEMP}/schema-${version}.json"
curl -s "http://127.0.0.1:${API_PORT}/swaggerapi/api/${version}" > "${file}"
[[ "$(grep "list of returned" "${file}")" ]]
[[ "$(grep "list of pods" "${file}")" ]]
[[ "$(grep "watch for changes to the described resources" "${file}")" ]]
fi

kube::test::clear_all
done

Expand Down
38 changes: 38 additions & 0 deletions pkg/api/conversion.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ package api
import (
"github.com/GoogleCloudPlatform/kubernetes/pkg/api/resource"
"github.com/GoogleCloudPlatform/kubernetes/pkg/conversion"
"github.com/GoogleCloudPlatform/kubernetes/pkg/fields"
"github.com/GoogleCloudPlatform/kubernetes/pkg/labels"
"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
)
Expand All @@ -28,12 +30,48 @@ import (
var Codec = runtime.CodecFor(Scheme, "")

func init() {
Scheme.AddDefaultingFuncs(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this necessary? I'd like to nuke this whole file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Smart defaulting for ListOptions deserialized from an incoming request. Rather than make all people using it set those to empty and risk doing something wrong.

----- Original Message -----

@@ -28,12 +30,48 @@ import (
var Codec = runtime.CodecFor(Scheme, "")

func init() {

  • Scheme.AddDefaultingFuncs(

Why is this necessary? I'd like to nuke this whole file.

Reply to this email directly or view it on GitHub:
https://github.com/GoogleCloudPlatform/kubernetes/pull/5763/files#r27089694

func(obj *ListOptions) {
obj.LabelSelector = labels.Everything()
obj.FieldSelector = fields.Everything()
},
)
Scheme.AddConversionFuncs(
func(in *util.Time, out *util.Time, s conversion.Scope) error {
// Cannot deep copy these, because time.Time has unexported fields.
*out = *in
return nil
},
func(in *string, out *labels.Selector, s conversion.Scope) error {
selector, err := labels.Parse(*in)
if err != nil {
return err
}
*out = selector
return nil
},
func(in *string, out *fields.Selector, s conversion.Scope) error {
selector, err := fields.ParseSelector(*in)
if err != nil {
return err
}
*out = selector
return nil
},
func(in *labels.Selector, out *string, s conversion.Scope) error {
if *in == nil {
return nil
}
*out = (*in).String()
return nil
},
func(in *fields.Selector, out *string, s conversion.Scope) error {
if *in == nil {
return nil
}
*out = (*in).String()
return nil
},
func(in *resource.Quantity, out *resource.Quantity, s conversion.Scope) error {
// Cannot deep copy these, because inf.Dec has unexported fields.
*out = *in.Copy()
Expand Down
8 changes: 8 additions & 0 deletions pkg/api/helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import (

"github.com/GoogleCloudPlatform/kubernetes/pkg/api/resource"
"github.com/GoogleCloudPlatform/kubernetes/pkg/conversion"
"github.com/GoogleCloudPlatform/kubernetes/pkg/fields"
"github.com/GoogleCloudPlatform/kubernetes/pkg/labels"
"github.com/GoogleCloudPlatform/kubernetes/pkg/util"

"github.com/davecgh/go-spew/spew"
Expand Down Expand Up @@ -63,6 +65,12 @@ var Semantic = conversion.EqualitiesOrDie(
func(a, b util.Time) bool {
return a.UTC() == b.UTC()
},
func(a, b labels.Selector) bool {
return a.String() == b.String()
},
func(a, b fields.Selector) bool {
return a.String() == b.String()
},
)

var standardResources = util.NewStringSet(
Expand Down
1 change: 1 addition & 0 deletions pkg/api/meta/restmapper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

// TODO: move everything in this file to pkg/api/rest
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@smarterclayton Sorry to refer this old PR and disturb you. I saw this todo and want to fix it, but I didn't catch why this file belongs to rest package, not meta package? Could you explain this? Thanks very much!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will try to respond tomorrow

package meta

import (
Expand Down
8 changes: 8 additions & 0 deletions pkg/api/meta/restmapper_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,18 @@ func (fakeCodec) DecodeInto([]byte, runtime.Object) error {

type fakeConvertor struct{}

func (fakeConvertor) Convert(in, out interface{}) error {
return nil
}

func (fakeConvertor) ConvertToVersion(in runtime.Object, _ string) (runtime.Object, error) {
return in, nil
}

func (fakeConvertor) ConvertFieldLabel(version, kind, label, value string) (string, string, error) {
return label, value, nil
}

var validCodec = fakeCodec{}
var validAccessor = resourceAccessor{}
var validConvertor = fakeConvertor{}
Expand Down
6 changes: 4 additions & 2 deletions pkg/api/register.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,12 @@ func init() {
&NamespaceList{},
&Secret{},
&SecretList{},
&DeleteOptions{},
&PersistentVolume{},
&PersistentVolumeList{},
&PersistentVolumeClaim{},
&PersistentVolumeClaimList{},
&DeleteOptions{},
&ListOptions{},
)
// Legacy names are supported
Scheme.AddKnownTypeWithName("", "Minion", &Node{})
Expand Down Expand Up @@ -90,8 +91,9 @@ func (*Namespace) IsAnAPIObject() {}
func (*NamespaceList) IsAnAPIObject() {}
func (*Secret) IsAnAPIObject() {}
func (*SecretList) IsAnAPIObject() {}
func (*DeleteOptions) IsAnAPIObject() {}
func (*PersistentVolume) IsAnAPIObject() {}
func (*PersistentVolumeList) IsAnAPIObject() {}
func (*PersistentVolumeClaim) IsAnAPIObject() {}
func (*PersistentVolumeClaimList) IsAnAPIObject() {}
func (*DeleteOptions) IsAnAPIObject() {}
func (*ListOptions) IsAnAPIObject() {}
2 changes: 1 addition & 1 deletion pkg/api/serialization_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ func TestList(t *testing.T) {
}

var nonRoundTrippableTypes = util.NewStringSet("ContainerManifest", "ContainerManifestList")
var nonInternalRoundTrippableTypes = util.NewStringSet("List")
var nonInternalRoundTrippableTypes = util.NewStringSet("List", "ListOptions")

func TestRoundTripTypes(t *testing.T) {
// api.Scheme.Log(t)
Expand Down
7 changes: 7 additions & 0 deletions pkg/api/testing/fuzzer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import (

"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
"github.com/GoogleCloudPlatform/kubernetes/pkg/api/resource"
"github.com/GoogleCloudPlatform/kubernetes/pkg/fields"
"github.com/GoogleCloudPlatform/kubernetes/pkg/labels"
"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
"github.com/GoogleCloudPlatform/kubernetes/pkg/types"
"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
Expand Down Expand Up @@ -88,6 +90,11 @@ func FuzzerFor(t *testing.T, version string, src rand.Source) *fuzz.Fuzzer {
j.ResourceVersion = strconv.FormatUint(c.RandUint64(), 10)
j.SelfLink = c.RandString()
},
func(j *api.ListOptions, c fuzz.Continue) {
// TODO: add some parsing
j.LabelSelector, _ = labels.Parse("a=b")
j.FieldSelector, _ = fields.ParseSelector("a=b")
},
func(j *api.PodPhase, c fuzz.Continue) {
statuses := []api.PodPhase{api.PodPending, api.PodRunning, api.PodFailed, api.PodUnknown}
*j = statuses[c.Rand.Intn(len(statuses))]
Expand Down
17 changes: 17 additions & 0 deletions pkg/api/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ package api

import (
"github.com/GoogleCloudPlatform/kubernetes/pkg/api/resource"
"github.com/GoogleCloudPlatform/kubernetes/pkg/fields"
"github.com/GoogleCloudPlatform/kubernetes/pkg/labels"
"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
"github.com/GoogleCloudPlatform/kubernetes/pkg/types"
"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
Expand Down Expand Up @@ -1168,6 +1170,21 @@ type DeleteOptions struct {
GracePeriodSeconds *int64 `json:"gracePeriodSeconds"`
}

// ListOptions is the query options to a standard REST list call, and has future support for
// watch calls.
type ListOptions struct {
TypeMeta `json:",inline"`

// A selector based on labels
LabelSelector labels.Selector
// A selector based on fields
FieldSelector fields.Selector
// If true, watch for changes to this list
Watch bool
// The resource version to watch (no effect on list yet)
ResourceVersion string
}

// Status is a return value for calls that don't return other objects.
// TODO: this could go in apiserver, but I'm including it here so clients needn't
// import both.
Expand Down
6 changes: 4 additions & 2 deletions pkg/api/unversioned.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,18 +40,20 @@ func PreV1Beta3(version string) bool {
return version == "v1beta1" || version == "v1beta2"
}

// TODO: remove me when watch is refactored
func LabelSelectorQueryParam(version string) string {
if PreV1Beta3(version) {
return "labels"
}
return "label-selector"
return "labelSelector"
}

// TODO: remove me when watch is refactored
func FieldSelectorQueryParam(version string) string {
if PreV1Beta3(version) {
return "fields"
}
return "field-selector"
return "fieldSelector"
}

// String returns available api versions as a human-friendly version string.
Expand Down
8 changes: 4 additions & 4 deletions pkg/api/v1beta1/conversion.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1494,7 +1494,7 @@ func init() {
}

// Add field conversion funcs.
err = newer.Scheme.AddFieldLabelConversionFunc("v1beta1", "pods",
err = newer.Scheme.AddFieldLabelConversionFunc("v1beta1", "Pod",
func(label, value string) (string, string, error) {
switch label {
case "name":
Expand All @@ -1514,7 +1514,7 @@ func init() {
// If one of the conversion functions is malformed, detect it immediately.
panic(err)
}
err = newer.Scheme.AddFieldLabelConversionFunc("v1beta1", "replicationControllers",
err = newer.Scheme.AddFieldLabelConversionFunc("v1beta1", "ReplicationController",
func(label, value string) (string, string, error) {
switch label {
case "name":
Expand All @@ -1529,7 +1529,7 @@ func init() {
// If one of the conversion functions is malformed, detect it immediately.
panic(err)
}
err = newer.Scheme.AddFieldLabelConversionFunc("v1beta1", "events",
err = newer.Scheme.AddFieldLabelConversionFunc("v1beta1", "Event",
func(label, value string) (string, string, error) {
switch label {
case "involvedObject.kind",
Expand All @@ -1551,7 +1551,7 @@ func init() {
// If one of the conversion functions is malformed, detect it immediately.
panic(err)
}
err = newer.Scheme.AddFieldLabelConversionFunc("v1beta1", "namespaces",
err = newer.Scheme.AddFieldLabelConversionFunc("v1beta1", "Namespace",
func(label, value string) (string, string, error) {
switch label {
case "status.phase":
Expand Down
6 changes: 4 additions & 2 deletions pkg/api/v1beta1/register.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,12 @@ func init() {
&NamespaceList{},
&Secret{},
&SecretList{},
&DeleteOptions{},
&PersistentVolume{},
&PersistentVolumeList{},
&PersistentVolumeClaim{},
&PersistentVolumeClaimList{},
&DeleteOptions{},
&ListOptions{},
)
// Future names are supported
api.Scheme.AddKnownTypeWithName("v1beta1", "Node", &Minion{})
Expand Down Expand Up @@ -97,8 +98,9 @@ func (*Namespace) IsAnAPIObject() {}
func (*NamespaceList) IsAnAPIObject() {}
func (*Secret) IsAnAPIObject() {}
func (*SecretList) IsAnAPIObject() {}
func (*DeleteOptions) IsAnAPIObject() {}
func (*PersistentVolume) IsAnAPIObject() {}
func (*PersistentVolumeList) IsAnAPIObject() {}
func (*PersistentVolumeClaim) IsAnAPIObject() {}
func (*PersistentVolumeClaimList) IsAnAPIObject() {}
func (*DeleteOptions) IsAnAPIObject() {}
func (*ListOptions) IsAnAPIObject() {}
14 changes: 14 additions & 0 deletions pkg/api/v1beta1/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -985,6 +985,20 @@ type DeleteOptions struct {
GracePeriodSeconds *int64 `json:"gracePeriodSeconds" description:"the duration in seconds to wait before deleting this object; defaults to a per object value if not specified; zero means delete immediately"`
}

// ListOptions is the query options to a standard REST list call
type ListOptions struct {
TypeMeta `json:",inline"`

// A selector based on labels
LabelSelector string `json:"labels" description:"a selector to restrict the list of returned objects by their labels; defaults to everything"`
// A selector based on fields
FieldSelector string `json:"fields" description:"a selector to restrict the list of returned objects by their fields; defaults to everything"`
// If true, watch for changes to the selected resources
Watch bool `json:"watch" description:"watch for changes to the described resources and return them as a stream of add, update, and remove notifications; specify resourceVersion"`
// The desired resource version to watch
ResourceVersion string `json:"resourceVersion" description:"when specified with a watch call, shows changes that occur after that particular version of a resource; defaults to changes from the beginning of history"`
}

// Status is a return value for calls that don't return other objects.
// TODO: this could go in apiserver, but I'm including it here so clients needn't
// import both.
Expand Down
Loading