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.

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

WIP: Add a Heartbeat object to the API #14735

Closed
wants to merge 1 commit into from

Conversation

gmarek
Copy link
Contributor

@gmarek gmarek commented Sep 29, 2015

Ref. #14733

@bgrant0607 PTAL. I purposefully didn't add this anywhere else, as those will be mechanical changes.

cc @wojtek-t

@@ -1368,6 +1368,13 @@ type NodeSpec struct {
Unschedulable bool `json:"unschedulable,omitempty"`
}

type Heartbeat struct {
// Time when this heartbeat was generated.
Copy link
Member

Choose a reason for hiding this comment

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

I think that you also need ObjectMeta - similarly to what we have in binding:
https://github.com/gmarek/kubernetes/blob/heartbeat_api/pkg/api/types.go#L1568

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think I need. There's a lot of various stuff, that's completely unimportant for a hearbeat (name, uuid, etc.). Binding is a different case, as it represents a binary relation, and we store one dimension as ObjectMeta, and second as ObjectReference. Here we need only one reference - I don't really care if it's ObjectMeta, or ObjectReference, but ObjectReference is smaller, and have everything we need (or at least everything I believe we need).

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I still think it's unnecessary, as I don't think that heartbeat would be something that "represent a persistent entity" or anything that defines "object entity". I might have missed something, as it's a long read.

@k8s-github-robot k8s-github-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Sep 29, 2015
@k8s-github-robot
Copy link

Labelling this PR as size/S

@k8s-bot
Copy link

k8s-bot commented Sep 29, 2015

Unit, integration and GCE e2e test build/test passed for commit 1bb7688.

@@ -1368,6 +1368,13 @@ type NodeSpec struct {
Unschedulable bool `json:"unschedulable,omitempty"`
}

type Heartbeat struct {
// Time when this heartbeat was generated.
Timestamp unversioned.Time `json:"timestamp,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What is the reason for 'no *stamp' rule? Seems pretty arbitrary to me.

@bgrant0607
Copy link
Member

Let's discuss on the issue.

@ncdc
Copy link
Member

ncdc commented Sep 29, 2015

@kubernetes/rh-cluster-infra

@mikedanese mikedanese added the area/api Indicates an issue on api area. label Sep 29, 2015
@k8s-github-robot k8s-github-robot added the kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API label Oct 9, 2015
@k8s-bot
Copy link

k8s-bot commented Dec 2, 2015

GCE e2e build/test failed for commit 1bb7688.

@@ -1353,14 +1353,14 @@ type EndpointsList struct {

// NodeSpec describes the attributes that a node is created with.
type NodeSpec struct {
// PodCIDR represents the pod IP range assigned to the node
// PodCIDR represents the pod IP range assigned to the node.
Copy link
Member

Choose a reason for hiding this comment

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

nit: peel these off into another PR? They're orthogonal to the point of this PR and easy to review.

@k8s-bot
Copy link

k8s-bot commented Dec 3, 2015

GCE e2e test build/test passed for commit 1bb7688.

@k8s-bot
Copy link

k8s-bot commented Dec 5, 2015

GCE e2e test build/test passed for commit 1bb7688.

@k8s-bot
Copy link

k8s-bot commented Dec 11, 2015

GCE e2e test build/test passed for commit 1bb7688.

@eparis
Copy link
Contributor

eparis commented Feb 1, 2016

@k8s-bot test this issue: #IGNORE

Tests have been pending for 24 hours

@k8s-bot
Copy link

k8s-bot commented Feb 2, 2016

GCE e2e test build/test passed for commit 1bb7688.

@bgrant0607
Copy link
Member

Closing. Can reopen when we decide what we want/need to do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api Indicates an issue on api area. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants