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

TCPSocket could not be used as it was not checked in validation #4883

Merged

Conversation

smarterclayton
Copy link
Contributor

Attempting to use it gave the error "must register one handler".
Added more tests for it.

@googlebot
Copy link

Thanks for your pull request.

It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA) at https://cla.developers.google.com/.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check the information on your CLA or see this help article on setting the email on your git commits.

Once you've done that, please reply here to let us know. If you signed the CLA as a corporation, please let us know the company's name.

@smarterclayton smarterclayton added kind/bug Categorizes issue or PR as related to a bug. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. labels Feb 27, 2015
@thockin thockin added cla: yes and removed cla: no labels Feb 27, 2015
@@ -454,6 +454,14 @@ func validateHTTPGetAction(http *api.HTTPGetAction) errs.ValidationErrorList {
return allErrors
}

func validateTCPSocketAction(tcp *api.TCPSocketAction) errs.ValidationErrorList {
allErrors := errs.ValidationErrorList{}
if tcp.Port.IntVal == 0 {
Copy link
Member

Choose a reason for hiding this comment

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

Do we want a comment about why this does not support named ports? E.g. "Named ports are probably going to be deprecated, so don't handle that 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.

Ill just make it correct - typo on my part

@thockin thockin self-assigned this Feb 27, 2015
Attempting to use it gave the error "must register one handler".
Added more tests for it.
@smarterclayton smarterclayton force-pushed the fix_tcpsocket_validation branch from 823377c to d3a5a48 Compare March 2, 2015 03:32
@googlebot googlebot added cla: no and removed cla: yes labels Mar 2, 2015
@smarterclayton
Copy link
Contributor Author

Comment addressed

@thockin
Copy link
Member

thockin commented Mar 2, 2015

LGTM

@thockin thockin added cla: yes lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed cla: no labels Mar 2, 2015
brendandburns added a commit that referenced this pull request Mar 2, 2015
TCPSocket could not be used as it was not checked in validation
@brendandburns brendandburns merged commit c7cbc5c into kubernetes:master Mar 2, 2015
@zmerlynn
Copy link
Member

zmerlynn commented Mar 2, 2015

This raced with #4927 and broke the build. Reverting this one arbitrarily.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants