Implement operator facade apis used to make the hook context #8173

Merged
merged 1 commit into from Dec 5, 2017

Conversation

Projects
None yet
3 participants
Owner

wallyworld commented Dec 5, 2017

Description of change

Add the caas operator facade APIs used to make a context factory and wire them up.

QA steps

smoketest a caas deployment

Looks fine apart from the change to TestWorkerDownloadsCharm, which I'd like to understand.

w, err := caasoperator.NewWorker(s.config)
c.Assert(err, jc.ErrorIsNil)
defer workertest.CleanKill(c, w)
s.settingsChanges <- struct{}{}
+ ctx.waitForHooks(c, []string{"config-changed"})
@axw

axw Dec 5, 2017

Member

why? what's this got to do with the test?

@wallyworld

wallyworld Dec 5, 2017

Owner

we need to wait for the hook to fire or else the method calls get the "APIAddresses" called when setting up the hook context

worker/caasoperator/mock_test.go
+}
+
+func (c *fakeClient) ProxySettings() (proxy.Settings, error) {
+ c.MethodCall(c, "APIAddresses", nil)
@axw

axw Dec 5, 2017

Member

ProxySettings

axw approved these changes Dec 5, 2017

please fix the ProxySettings stub name

Owner

wallyworld commented Dec 5, 2017

$$merge$$

Contributor

jujubot commented Dec 5, 2017

Status: merge request accepted. Url: http://ci.jujucharms.com/job/github-merge-juju

Contributor

jujubot commented Dec 5, 2017

Build failed: Tests failed
build url: http://ci.jujucharms.com/job/github-merge-juju/635

Owner

wallyworld commented Dec 5, 2017

$$merge$$

Contributor

jujubot commented Dec 5, 2017

Status: merge request accepted. Url: http://ci.jujucharms.com/job/github-merge-juju

Contributor

jujubot commented Dec 5, 2017

Build failed: Tests failed
build url: http://ci.jujucharms.com/job/github-merge-juju/637

Owner

wallyworld commented Dec 5, 2017

$$merge$$

Contributor

jujubot commented Dec 5, 2017

Status: merge request accepted. Url: http://ci.jujucharms.com/job/github-merge-juju

@jujubot jujubot merged commit 3c2eceb into juju:develop Dec 5, 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