julz / cube

a kube backend for cf, because k-why not
21 stars 8 forks source link

Wire sync and registry jobs #8

Closed herrjulz closed 6 years ago

herrjulz commented 6 years ago

This PR wires the sync and registry job and, thus, removes the hardcoded busybox from the dropletToImageUri function.

This PR contains the following changes:

implement dropletToImageUrl()

Based on the Desired App, this code:

The cfclient, http client, and other required parameters are passed from the main function. Therefore the Convert function signiture is enhanced.

Introduce cfclient

As the sync loop requires to download droplets from the cloundcontroller/blobstore and the internal clound controller api endpoint cannot be resolved, the public api endpoint is used. The cfclient simplifies the process to get the bearer token by just providing the cf admin user and password.

The cfclient code reuses the client.go (and some more required code) from https://github.com/cloudfoundry-community/go-cfclient and enhances the client code with the GetDropletByAppGuid method.

Introduce models.go

Having all domain types in a model.go has the following advantages:

More about this approach can be read here.

Enhance sync CLI

The sync CLI add further parameters to setup the cfclient, which is required to download the droplets. Moreover, the CLI was enhanced to consume a config YAML file instead of passing CLI flags.

Update Tests

The tests were updated according the code changes.

Update sink README

Add some output in the registry when droplets are added

The code was tested on a local Bosh-Lite and a Minikube cluster (also in combination with the cube-release) (here is the PR for the cube-release)

julz commented 6 years ago

any chance we could tidy the commits up a bit here before we merge? the merge commit at the end makes it a bit hard to see what actually happened and I can't find a single commit where dropletToImage went in that doesn't also include lots of dependency additions. Maybe a commit to introduce the new dependencies and a commit to implement dropletToImage or something?

herrjulz commented 6 years ago

@julz I cleaned up the commit history into 4 commits:

herrjulz commented 6 years ago

@julz I rewrote the error handling for the Convert function as discussed. It panics inside dropletToImageUrl and ConvergeOnce recovers and logs the error. Furthermore:

julz commented 6 years ago

cool, in retrospect I think letting the panic escape from Convert might be a bit bad (since it's an exported function), not a big deal though we can always tidy that up later. Gonna merge.