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

Add first version of Pod proto #30

Merged
merged 10 commits into from
Nov 26, 2019
Merged

Add first version of Pod proto #30

merged 10 commits into from
Nov 26, 2019

Conversation

mfpierre
Copy link
Contributor

@mfpierre mfpierre commented Nov 6, 2019

No description provided.

@mfpierre mfpierre force-pushed the mfpierre/collect-pods branch from c06680b to df92f7a Compare November 7, 2019 16:39
@mfpierre mfpierre force-pushed the mfpierre/collect-pods branch from df92f7a to a2b0dc5 Compare November 7, 2019 16:40
proto/process/agent.proto Outdated Show resolved Hide resolved
string IP = 4;
string nominatedNodeName = 5;
string nodeName = 6;
int32 restartCount = 7;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit, does this need to be an int32? is there a smaller type that we could use?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

}

message OwnerReference {
string apiVersion = 5;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit, if we're intentionally omitting field 2, can we mark it as reserved?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

message ContainerStatus {
string name = 1;
string containerID = 2;
bool ready = 3;
Copy link
Contributor

Choose a reason for hiding this comment

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

are ready and started mutually exclusive? It might be better to use an enum for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the slight difference is that started is for the startup probe and ready the readiness probe, so you could be in started:true, ready:false and later started:true, ready:true

Copy link
Contributor Author

Choose a reason for hiding this comment

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

edit: removed the started for now as it's k8s > 1.16

string namespace = 2;
int64 age = 3;
string IP = 4;
string nominatedNodeName = 5;
Copy link
Contributor

Choose a reason for hiding this comment

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

would it be better to use the node UUID? My understanding is that it's technically possible for node names to be re-used within an org if you have multiple kubernetes clusters, and that would make getting the node for a pod tricky.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, I don't think we have this info at the pod level though, with cluster-name + hostname we should be fine though 🤔 (although I'm not sure how we can enforce cluster-name to be unique between clusters)

string nodeName = 6;
int32 restartCount = 7;
repeated ContainerStatus containerStatuses = 8;
repeated string labels = 9;
Copy link
Contributor

Choose a reason for hiding this comment

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

Are labels and tags the same thing or different? I currently see both in here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes labels are the pod labels and tags are our internal tags stored in the tagger for this particular pod object some info will be duplicated with what we collect from pod already but some will not (ie kube_service)

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. Maybe put a comment on tag to mark it for internal use so people don't ask this question again.

@mfpierre mfpierre force-pushed the mfpierre/collect-pods branch from cdfeb40 to 2994d48 Compare November 20, 2019 15:42
@mfpierre mfpierre changed the title [WIP] Add first version of Pod proto Add first version of Pod proto Nov 22, 2019
@mfpierre mfpierre marked this pull request as ready for review November 22, 2019 13:58
Copy link
Contributor

@shang-wang shang-wang left a comment

Choose a reason for hiding this comment

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

\o/

proto/process/agent.proto Outdated Show resolved Hide resolved
@mfpierre mfpierre merged commit 1430b85 into master Nov 26, 2019
@mfpierre mfpierre deleted the mfpierre/collect-pods branch November 26, 2019 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants