Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(314)

Delta Between Two Patch Sets: worker/provisioner/container_initialisation.go

Issue 25040043: Refactor container provisioner (Closed)
Left Patch Set: - Created 10 years, 4 months ago
Right Patch Set: - Created 10 years, 4 months ago
Left:
Right:
Use n/p to move between diff chunks; N/P to move between comments. Please Sign in to add in-line comments.
Jump to:
Left: Side by side diff | Download
Right: Side by side diff | Download
LEFTRIGHT
1 // Copyright 2012, 2013 Canonical Ltd. 1 // Copyright 2012, 2013 Canonical Ltd.
2 // Licensed under the AGPLv3, see LICENCE file for details. 2 // Licensed under the AGPLv3, see LICENCE file for details.
3 3
4 package provisioner 4 package provisioner
5 5
6 import ( 6 import (
7 "fmt" 7 "fmt"
8 "sync/atomic" 8 "sync/atomic"
9 9
10 "launchpad.net/juju-core/agent" 10 "launchpad.net/juju-core/agent"
11 "launchpad.net/juju-core/instance" 11 "launchpad.net/juju-core/instance"
12 apiprovisioner "launchpad.net/juju-core/state/api/provisioner" 12 apiprovisioner "launchpad.net/juju-core/state/api/provisioner"
13 "launchpad.net/juju-core/state/api/watcher" 13 "launchpad.net/juju-core/state/api/watcher"
14 "launchpad.net/juju-core/worker" 14 "launchpad.net/juju-core/worker"
15 ) 15 )
16 16
17 // ContainerSetup is a StringsWatchHandler that is notified when containers of 17 // ContainerSetup is a StringsWatchHandler that is notified when containers of
18 // the specified type are created on the given machine. It will set up the 18 // the specified type are created on the given machine. It will set up the
19 // machine to be able to create containers and start a provisioner. 19 // machine to be able to create containers and start a provisioner.
20 type ContainerSetup struct { 20 type ContainerSetup struct {
21 runner worker.Runner 21 runner worker.Runner
22 containerType instance.ContainerType 22 containerType instance.ContainerType
23 provisioner *apiprovisioner.State 23 provisioner *apiprovisioner.State
24 machine *apiprovisioner.Machine 24 machine *apiprovisioner.Machine
25 config agent.Config 25 config agent.Config
26 26
27 // Save the workerName so the worker thread can be stopped. 27 // Save the workerName so the worker thread can be stopped.
28 workerName string 28 workerName string
29 // Save the watcher so it can be stopped.
30 watcher watcher.StringsWatcher
31 // setupDone is non zero if the container setup has been invoked. 29 // setupDone is non zero if the container setup has been invoked.
32 setupDone int32 30 setupDone int32
33 } 31 }
34 32
35 // NewContainerSetupHandler returns a StringsWatchHandler which is notified when 33 // NewContainerSetupHandler returns a StringsWatchHandler which is notified when
36 // containers are created on the given machine. 34 // containers are created on the given machine.
37 func NewContainerSetupHandler(runner worker.Runner, workerName string, container instance.ContainerType, 35 func NewContainerSetupHandler(runner worker.Runner, workerName string, container instance.ContainerType,
38 machine *apiprovisioner.Machine, provisioner *apiprovisioner.State, 36 machine *apiprovisioner.Machine, provisioner *apiprovisioner.State,
39 config agent.Config) worker.StringsWatchHandler { 37 config agent.Config) worker.StringsWatchHandler {
40 38
41 return &ContainerSetup{ 39 return &ContainerSetup{
42 runner: runner, 40 runner: runner,
43 containerType: container, 41 containerType: container,
44 machine: machine, 42 machine: machine,
45 provisioner: provisioner, 43 provisioner: provisioner,
46 config: config, 44 config: config,
47 workerName: workerName, 45 workerName: workerName,
48 } 46 }
49 } 47 }
50 48
51 // SetUp is defined on the StringsWatchHandler interface. 49 // SetUp is defined on the StringsWatchHandler interface.
52 func (cs *ContainerSetup) SetUp() (watcher watcher.StringsWatcher, err error) { 50 func (cs *ContainerSetup) SetUp() (watcher watcher.StringsWatcher, err error) {
53 » if cs.watcher, err = cs.machine.WatchContainers(cs.containerType); err ! = nil { 51 » if watcher, err = cs.machine.WatchContainers(cs.containerType); err != n il {
54 return nil, err 52 return nil, err
55 } 53 }
56 » return cs.watcher, nil 54 » return watcher, nil
57 } 55 }
58 56
59 // Handle is called whenever containers change on the machine being watched. 57 // Handle is called whenever containers change on the machine being watched.
60 // All machines start out with so containers so the first time Handle is called, 58 // All machines start out with so containers so the first time Handle is called,
61 // it will be because a container has been added. 59 // it will be because a container has been added.
fwereade 2013/11/13 10:26:11 Are you sure? Don't we get an empty initial event?
wallyworld 2013/11/14 05:25:08 I added a check for the initial event to discard i
62 func (cs *ContainerSetup) Handle(containerIds []string) error { 60 func (cs *ContainerSetup) Handle(containerIds []string) error {
61 // Consume the initial watcher event.
62 if len(containerIds) == 0 {
63 return nil
64 }
65
63 // This callback must only be invoked once. Stopping the watcher 66 // This callback must only be invoked once. Stopping the watcher
64 // below should be sufficient but I'm paranoid. 67 // below should be sufficient but I'm paranoid.
65 if atomic.LoadInt32(&cs.setupDone) != 0 { 68 if atomic.LoadInt32(&cs.setupDone) != 0 {
66 return nil 69 return nil
67 } 70 }
68 atomic.StoreInt32(&cs.setupDone, 1) 71 atomic.StoreInt32(&cs.setupDone, 1)
72
69 logger.Tracef("initial container setup with ids: %v", containerIds) 73 logger.Tracef("initial container setup with ids: %v", containerIds)
70 // We only care about the initial container creation. 74 // We only care about the initial container creation.
71 » // This handler has done its job so stop it. 75 » // This worker has done its job so stop it.
72 » cs.watcher.Stop() 76 » // We do not expect there will be an error, and there's not much we can do anyway.
fwereade 2013/11/13 10:26:11 error? I can see an argument against, but it shoul
wallyworld 2013/11/14 05:25:08 I logged a warning
73 » cs.runner.StopWorker(cs.workerName) 77 » if err := cs.runner.StopWorker(cs.workerName); err != nil {
78 » » logger.Warningf("stopping machine agent container watcher: %v", err)
79 » }
74 if err := cs.ensureContainerDependencies(); err != nil { 80 if err := cs.ensureContainerDependencies(); err != nil {
william.reade 2013/11/18 15:30:58 I still think this is better done lazily inside th
75 return fmt.Errorf("setting up container dependnecies on host mac hine: %v", err) 81 return fmt.Errorf("setting up container dependnecies on host mac hine: %v", err)
76 } 82 }
77 var provisionerType ProvisionerType 83 var provisionerType ProvisionerType
78 switch cs.containerType { 84 switch cs.containerType {
79 case instance.LXC: 85 case instance.LXC:
80 provisionerType = LXC 86 provisionerType = LXC
81 case instance.KVM: 87 case instance.KVM:
82 provisionerType = KVM 88 provisionerType = KVM
83 default: 89 default:
84 return fmt.Errorf("invalid container type %q", cs.containerType) 90 return fmt.Errorf("invalid container type %q", cs.containerType)
85 } 91 }
86 » return startProvisionerWorker(cs.runner, provisionerType, cs.provisioner , cs.config) 92 » return StartProvisioner(cs.runner, provisionerType, cs.provisioner, cs.c onfig)
fwereade 2013/11/13 10:26:11 Should we have mirror logic in the provisioner tha
wallyworld 2013/11/14 05:25:08 We don't need this IMO. If there are no more conta
87 } 93 }
88 94
89 // TearDown is defined on the StringsWatchHandler interface. 95 // TearDown is defined on the StringsWatchHandler interface.
90 func (cs *ContainerSetup) TearDown() error { 96 func (cs *ContainerSetup) TearDown() error {
91 // Nothing to do here. 97 // Nothing to do here.
92 return nil 98 return nil
93 } 99 }
94 100
95 func (cs *ContainerSetup) ensureContainerDependencies() error { 101 func (cs *ContainerSetup) ensureContainerDependencies() error {
96 // TODO(wallyworld) - install whatever dependencies are required to supp ort starting containers 102 // TODO(wallyworld) - install whatever dependencies are required to supp ort starting containers
97 return nil 103 return nil
fwereade 2013/11/13 10:26:11 suggestion -- keep this type focused on runner man
wallyworld 2013/11/14 05:25:08 I still think we want the int32 check just in case
98 } 104 }
99 105
100 // Override for testing. 106 // Override for testing.
101 var startProvisioner = startProvisionerWorker 107 var StartProvisioner = startProvisionerWorker
102 108
103 // startProvisioner kicks off a provisioner task responsible for creating contai ners 109 // startProvisionerWorker kicks off a provisioner task responsible for creating containers
104 // of the specified type on the machine. 110 // of the specified type on the machine.
105 func startProvisionerWorker(runner worker.Runner, provisionerType ProvisionerTyp e, 111 func startProvisionerWorker(runner worker.Runner, provisionerType ProvisionerTyp e,
106 provisioner *apiprovisioner.State, config agent.Config) error { 112 provisioner *apiprovisioner.State, config agent.Config) error {
107 113
108 workerName := fmt.Sprintf("%s-provisioner", provisionerType) 114 workerName := fmt.Sprintf("%s-provisioner", provisionerType)
109 // The provisioner task is created after a container record has already been added to the machine. 115 // The provisioner task is created after a container record has already been added to the machine.
110 // It will see that the container does not have an instance yet and crea te one. 116 // It will see that the container does not have an instance yet and crea te one.
111 return runner.StartWorker(workerName, func() (worker.Worker, error) { 117 return runner.StartWorker(workerName, func() (worker.Worker, error) {
112 return NewProvisioner(provisionerType, provisioner, config), nil 118 return NewProvisioner(provisionerType, provisioner, config), nil
william.reade 2013/11/18 15:30:58 This is all still kinda messed up (nothing to do w
113 }) 119 })
114 } 120 }
LEFTRIGHT

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b