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

Addition of a KubeLatentWorker #3155

Closed
wants to merge 1 commit into from

Conversation

Zempashi
Copy link
Contributor

I added a kubernetes Latent Worker to buildbot.
This is not done yet.

The Kubernetes API is quite large, I don't want to add a parameter for each possibility (volume, secrets, service-account, etc...), so I went with the all API offered by the python client, adding a layer to render variable that comme from buildbot. This could be arguable, I'm happy to discuss.

I would like early review for the direction to take on this.
Before be ready to merge I need to develop the stop_instance(fast=False) and everything else you tell me.
I agree for more unit test, more integration test (may be the same conformance test as the hyperLatentWorker does), more doc.

I have idea for developping followStartupLogs option but it's not materialized yet.

@tardyp
Copy link
Member

tardyp commented Apr 27, 2017

@Zempashi I have not looked the PR in details yet, but a first comment come to my mind.

This is using the python binding for kubernetes REST api.

Most of the time, Buildbot only needs one or two REST api calls, and it is eventually much simpler to just use the REST api directly.

This is how MarathonLatentWorker is implemented, and I think it is much easier to develop and test, using httpclientservice.

@Zempashi
Copy link
Contributor Author

Agree, I'll do this

defer.returnValue(res)

@defer.inlineCallbacks
def recursive_render(self, obj, props):
Copy link
Member

Choose a reason for hiding this comment

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

This would be so much simpler if we would just use the REST api.

We would just use json like objects that are already deeply renderable.

cls.dependency_error()
job_name = '%(prop:buildername)s-%(prop:buildnumber)s'
return client.V1Job(
metadata=client.V1ObjectMeta(name=job_name),
Copy link
Member

Choose a reason for hiding this comment

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

again, here we replace client.*( by dict( and we don't use the unneeded typing system from the python binding.

If we make typing error, the error will be at runtime as well, but done by the k8s server instead of the python binding. That does not make much of a difference.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For now, I have a couple of call : create_namespace_job and delete_namespace_job.
But this call create job and job created pods (docker containers). That's how kubernetes handle short live container
That's also why I miss the stop_container(fast=False) and followStartupLogs. I need to find pod from a job.
This would use list_namespaced_pod (https://github.com/kubernetes-incubator/client-python/blob/master/kubernetes/docs/CoreV1Api.md#list_namespaced_pod) (with long polling to wait for pod creation or deletion).
'FollowStartupLog' needs read_namespaced_pod_log (https://github.com/kubernetes-incubator/client-python/blob/master/kubernetes/docs/CoreV1Api.md#read_namespaced_pod_log) with streaming capabilities.
I don't have any other feature in minds (may be managing password with 'secrets'), but each would add a bunch of call. This is basically redevelopping the python client.

But again, I agree the render thing is really ugly. I've read somewere that the python client can also take yaml string (need to figure out how). The string would be renderable by default, without the hassle of developping each call.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll do as you want:

  • trying string with the kubernetes client
  • pure http call

@@ -10,6 +10,48 @@ Release Notes

.. towncrier release notes start

Buildbot ``0.9.6-35-gfb26e170d`` ( ``2017-04-27`` )
Copy link
Member

Choose a reason for hiding this comment

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

You need to make sure not to commit that file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Arghh.

It keeps adding to the commit index (I don't do commit -a or else). Do you know why ?
Surely because I run make -C master/docs SPHINXOPTS=-W spelling to compile docs

Does another command exists ?

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.

None yet

2 participants