Skip to content

Cleanup to client initialization in Kubelet#3270

Merged
erictune merged 3 commits intokubernetes:masterfrom
smarterclayton:kubelet_fixes
Jan 7, 2015
Merged

Cleanup to client initialization in Kubelet#3270
erictune merged 3 commits intokubernetes:masterfrom
smarterclayton:kubelet_fixes

Conversation

@smarterclayton
Copy link
Copy Markdown
Contributor

  • Watching invalid label/field selectors should error
  • Watch locks restrict ability to test certain things
  • Kubelet does not properly distinguish between sources, prepare for API change

Extracted from #846, @erictune this is all stuff you'll hit

@lavalamp review please

@thockin
Copy link
Copy Markdown
Member

thockin commented Jan 7, 2015

As people are fixing all the watch-related bugs, I'd really appreciate someone to take a look at kube2sky.go and see what mistakes I made :)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

KubeClient?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is not a warning, since people use kubelets without an apiserver on purpose.
If you want to warn, then warn when apiServerList is non-empty, and it can't setup a client.

@erictune
Copy link
Copy Markdown
Contributor

erictune commented Jan 7, 2015

@lavalamp can I take this review off your hands?

@lavalamp lavalamp assigned erictune and unassigned lavalamp Jan 7, 2015
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This would be a fine place for a warning. Actually, a fatal seems fine with me.

@erictune
Copy link
Copy Markdown
Contributor

erictune commented Jan 7, 2015

LGTM

erictune added a commit that referenced this pull request Jan 7, 2015
Cleanup to client initialization in Kubelet
@erictune erictune merged commit 0d4d1e2 into kubernetes:master Jan 7, 2015
@smarterclayton smarterclayton deleted the kubelet_fixes branch February 11, 2015 02:21
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.

4 participants