Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Support k8s deployment controllers #8191
Conversation
|
!!build!! |
| @@ -14,8 +14,17 @@ type Broker interface { | ||
| // a charm for the specified application. | ||
| EnsureOperator(appName, agentPath string, config *OperatorConfig) error | ||
| + // EnsureService creates or updates a service for pods with the given spec. | ||
| + EnsureService(appName, unitSpec string, numUnits int32, config ResourceConfig) error |
axw
Dec 8, 2017
Member
any particular reason to use int32 and not int here? seems a bit unusual for an API at this level
| + | ||
| +import "strings" | ||
| + | ||
| +// TODO(cass) - use a broker specific schema and then add tests |
| + } | ||
| + cleanups = append(cleanups, func() { k.deleteDeployment(appName) }) | ||
| + | ||
| + var port *v1.ContainerPort |
axw
Dec 8, 2017
Member
as discussed on IRC, I think we should expose all ports in the service, and just have the first one be the one that's exposed via ingress
| + // Now we have processed all the changed units, notify the | ||
| + // worker of those that are still alive. | ||
| + if aw.brokerManagedUnits { | ||
| + aw.aliveUnitsChan <- aliveUnits.Values() |
axw
Dec 8, 2017
Member
No naked channel sends. What happens during agent termination? And also, this prevents processing further unit changes until the channel is ready to receive.
I think you want to do something like this:
var aliveUnitsChan chan []string
...
select {
...
case units, ok := <-uw.Changes():
...
if aw.brokerManagedUnits {
aliveUnitsChan = aw.aliveUnitsChan
}
case aliveUnitsChan <- aliveUnits.Values():
aliveUnitsChan = nil
}| + | ||
| +import ( | ||
| + "github.com/juju/errors" | ||
| + "github.com/juju/juju/watcher" |
| + defer workertest.CleanKill(c, w) | ||
| + | ||
| + s.applicationGetter.CheckCallNames(c, "WatchApplications") | ||
| + //s.unitGetter.CheckCallNames(c, "WatchUnits") |
|
$$merge$$ |
|
Status: merge request accepted. Url: http://ci.jujucharms.com/job/github-merge-juju |
jujubot
merged commit 9afaa1f
into
juju:develop
Dec 8, 2017
1 check passed
continuous-integration/jenkins/pr-merge
This commit looks good
Details
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
wallyworld commentedDec 8, 2017
Description of change
Now when a CAAS charm is deployed, we:
There's also initial infrastructure to allow aspects of the deployment to be user configured once we support recording those settings for the app.
QA steps
deploy a caas model on a k8s with a running nginx ingress controller
juju deploy gitlab
->check 1 pod
add/remove units
-> check pod count matches
remove all units
-> check deployment controller and service are deleted
check ingress works (assuming external address is hardwired into the code)