Skip to content

Add support for thirdparty resource watch/create/put and delete operations#216

Merged
mbohlool merged 13 commits into
kubernetes-client:masterfrom
jonathan-kosgei:dev
May 18, 2017
Merged

Add support for thirdparty resource watch/create/put and delete operations#216
mbohlool merged 13 commits into
kubernetes-client:masterfrom
jonathan-kosgei:dev

Conversation

@jonathan-kosgei

@jonathan-kosgei jonathan-kosgei commented May 9, 2017

Copy link
Copy Markdown
Contributor

Fixes #201

Support third-party resources

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label May 9, 2017
@codecov-io

codecov-io commented May 9, 2017

Copy link
Copy Markdown

Codecov Report

Merging #216 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #216   +/-   ##
=======================================
  Coverage   94.56%   94.56%           
=======================================
  Files           9        9           
  Lines         681      681           
=======================================
  Hits          644      644           
  Misses         37       37

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7a6bae2...e469509. Read the comment docs.

@jonathan-kosgei

Copy link
Copy Markdown
Contributor Author

How do I name the classes in swagger? So that we don't get something like DefaultAPi as the api_instance object?

@mbohlool

mbohlool commented May 9, 2017

Copy link
Copy Markdown
Contributor

@jonathan-kosgei add a tag to operations. Look at our swagger.json for examples.

Comment thread scripts/preprocess_spec.py Outdated
with open('thirdpartypaths.json', 'r') as third_party_spec_file:
third_party_spec = json.loads(third_party_spec_file.read())
for path in third_party_spec.keys():
spec['paths'][path] = third_party_spec[path]

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.

Can you just add a validation here to make sure we are not overwriting an existing path?

@mbohlool

mbohlool commented May 9, 2017

Copy link
Copy Markdown
Contributor

one nit comment. Let me know when you added the tags for better naming and one example. This looks good so far.

(btw, use git commit --amend for small commits that should be part of the previous commit. you can merge that one small commit by git rebase -i HEAD~2)

@jonathan-kosgei

Copy link
Copy Markdown
Contributor Author

Awesome tip. Thanks. Pushing changes, including a example. I'll add a few more as I go along.

Comment thread examples/create_thirdparty_resource.py Outdated
from kube_resource.rest import ApiException
from pprint import pprint

with open('/var/run/secrets/kubernetes.io/serviceaccount/token', 'r') as token_file:

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.

Please use load_kube_config like other examples.

Comment thread examples/create_thirdparty_resource.py Outdated
resource = 'repos'
fqdn = 'git.k8s.com'

body = json.loads('{ "apiVersion": "git.k8s.com/v1", "kind": "RePo", "metadata": { "name": "blog-repo" }, "repo": "github.com/user/my-blog", "oauth": 123456789, "branch": "master", "key": "user-git-deploy-secret", "path": "/path/in/volume", "image": "image to run job in", "then": "hugo --destination=/home/user/hugosite/public", "pvc": "persistent-volume-claim", "username": "repo username", "password": "repo password"}')

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.

I would just make it a python map. The syntax is almost identical to json.

Comment thread examples/create_thirdparty_resource.py Outdated
kube_resource.configuration.api_key['Authorization'] = token
kube_resource.configuration.api_key_prefix['Authorization'] = 'Bearer'
kube_resource.configuration.ssl_ca_cert = '/var/run/secrets/kubernetes.io/serviceaccount/ca.crt'
api_instance = kube_resource.DefaultApi()

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.

I thought you renamed DefaultAPi using tags?

Comment thread scripts/thirdpartypaths.json Outdated
"summary": "Gets Resources",
"description": "Returns a list of Resources",
"tags": [
"thirdparty_v1"

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.

Name Suggestion : third_party_resource?

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.

third_party_resources

@mbohlool

Copy link
Copy Markdown
Contributor

Look like there are some formatting error too. Use script/update-pep8.sh

@jonathan-kosgei

Copy link
Copy Markdown
Contributor Author

On it.

@pvdvreede

Copy link
Copy Markdown

This is something I would really like so that I can use the python client to create a custom operator. Will this be merged/released soon?

Is there a current way to watch a specific third party resource I create in version 2.0 as a stop gap?

@mbohlool

Copy link
Copy Markdown
Contributor

@jonathan-kosgei any update on this? Can I help with anything?
I too hope this merge in soon.

@mbohlool

Copy link
Copy Markdown
Contributor

The code looks good. I will Squash and merge after tests passed.

@jonathan-kosgei

Copy link
Copy Markdown
Contributor Author

Hi @pvdvreede Sorry, got caught up with work, fixed

@pvdvreede

Copy link
Copy Markdown

Great! Thanks very much! Hopefully a new release can be done soon?

@mbohlool

Copy link
Copy Markdown
Contributor

@jonathan-kosgei The client needs to be updated with the new yaml changes. I will send a follow up PR tomorrow for that.
@pvdvreede We need to finalize the interface and add some more tests before we cut a release. Would be nice if you can install the client from the master and give this a try and maybe add some examples so we know the interface is good enough. I will do some work on this soon.

@mbohlool mbohlool merged commit dab09e0 into kubernetes-client:master May 18, 2017
@jonathan-kosgei jonathan-kosgei deleted the dev branch May 18, 2017 08:00
@pvdvreede

Copy link
Copy Markdown

Thanks @mbohlool I gave this a go last night and used the master branch. It errored saying there was no ThirdPartyResources attribute on kubernetes module. So I am not sure how to get the api instance. Does the example need updating/changing?

@jonathan-kosgei

jonathan-kosgei commented May 18, 2017

Copy link
Copy Markdown
Contributor Author

I think probably the client needs to be regenerated? @mbohlool
Though I think that would depend on the release planning already in place?
You can clone the code locally and run the update-client script in the scripts folder. In the meantime take a look at https://github.com/jonathan-kosgei/kubeResource, I've tested that, and used it as the base for this pr

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cncf-cla: yes Indicates the PR's author has signed the CNCF CLA.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants