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

Added automatic inclusion of object kind and api version for create requ... #58

Merged
merged 3 commits into from
Apr 7, 2015

Conversation

stephenjlevy
Copy link
Contributor

Creating entities was failing because the spec requires the entity kind and apiversion. Added code to automatically include these fields.

@abonas
Copy link
Member

abonas commented Apr 1, 2015

@stephenjlevy thanks for the PR.
What error were you getting from the server prior to the fix?
I assume it is relevant to this issue?
kubernetes/kubernetes#3000

@stephenjlevy
Copy link
Contributor Author

@abonas I assumed that the "kind" field was being automatically included since the request knew the entity type, but it wasn't. I was seeing the following error when trying to start a service without specifying "kind": "Service" or the api version.

/usr/local/lib/ruby/gems/2.0.0/gems/kubeclient-0.1.8/lib/kubeclient/common.rb:15:inrescue in handle_exception': Service "" is invalid: metadata.name: required value, spec.port: invalid value '0': must be greater than 0 and less than 65536, spec.protocol: required value, spec.sessionAffinity: required value`

@abonas
Copy link
Member

abonas commented Apr 4, 2015

@abonas I assumed that the "kind" field was being automatically included since the request knew the entity type, but it wasn't. I was seeing the following error when trying to start a service without specifying "kind": "Service" or the api version.

/usr/local/lib/ruby/gems/2.0.0/gems/kubeclient-0.1.8/lib/kubeclient/common.rb:15:inrescue in handle_exception': Service "" is invalid: metadata.name: required value, spec.port: invalid value '0': must be greater than 0 and less than 65536, spec.protocol: required value, spec.sessionAffinity: required value`

afaik both kind and api version are filled automatically by server side.
I tried to create for example a namespace without specifying them and the request succeeds.

{
"metadata": {
"name": "staging"
},
"spec": {},
"labels": {
"name": "staging"
}
}

It seems that there were other fields that were missing in your example based on the message that you pasted above - name, port, protocol and sessionAffinity. Can you post the example json /how do you construct the service object?

@abonas
Copy link
Member

abonas commented Apr 4, 2015

@stephenjlevy I think I might understand now what you're experiencing, I opened a question regarding it, please see here, lets follow the responses there and based on that see if your PR needs to be merged or not.
kubernetes/kubernetes#6439

@abonas
Copy link
Member

abonas commented Apr 6, 2015

@stephenjlevy , please add a comment in code that those additions are needed till kubernetes/kubernetes#6439 is resolved, and I'll merge the PR.

@stephenjlevy
Copy link
Contributor Author

Travis is failing on new regular expression checking that was just added to rubocop 0.30.0 (released earlier today). The regex errors are in upstream code, not in code related to this PR.

@abonas
Copy link
Member

abonas commented Apr 7, 2015

@stephenjlevy , I fixed that here:
#62
you can rebase.
I am going to limit rubocop version for future to gain more control over it.

@stephenjlevy
Copy link
Contributor Author

@abonas I have rebased

abonas added a commit that referenced this pull request Apr 7, 2015
Added automatic inclusion of object kind and api version for create requ...
@abonas abonas merged commit 8987ffb into ManageIQ:master Apr 7, 2015
cben added a commit to cben/kubeclient that referenced this pull request May 31, 2018
Can we revert ManageIQ#58?
Seems kubernetes/kubernetes#6439 fixed - from which kubernetes version?

kubernetes/kubernetes#10200 landed in v1.1.0.
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.

2 participants