Skip to content

Commit

Permalink
Fix controller terminating due to caasapplicationprovisioner.
Browse files Browse the repository at this point in the history
- Fix controller terminating due to caasapplicationprovisioner updating
  controller colocated sidecar charm.
- Fix startup shell scripts.
- Fix proxy error not surfacing.
  • Loading branch information
hpidcock committed Jun 2, 2023
1 parent a35072d commit eae516e
Show file tree
Hide file tree
Showing 22 changed files with 191 additions and 34 deletions.
16 changes: 16 additions & 0 deletions api/controller/caasapplicationprovisioner/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -473,3 +473,19 @@ func (c *Client) SetProvisioningState(appName string, state params.CAASApplicati
}
return nil
}

// ProvisionerConfig returns the provisoner's configuration.
func (c *Client) ProvisionerConfig() (params.CAASApplicationProvisionerConfig, error) {
var result params.CAASApplicationProvisionerConfigResult
err := c.facade.FacadeCall("ProvisionerConfig", nil, &result)
if err != nil {
return params.CAASApplicationProvisionerConfig{}, err
}
if result.Error != nil {
return params.CAASApplicationProvisionerConfig{}, result.Error
}
if result.ProvisionerConfig == nil {
return params.CAASApplicationProvisionerConfig{}, nil
}
return *result.ProvisionerConfig, nil
}
24 changes: 24 additions & 0 deletions api/controller/caasapplicationprovisioner/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -611,3 +611,27 @@ func (s *provisionerSuite) TestDestroyUnitsMismatchResults(c *gc.C) {
c.Assert(err.Error(), gc.Equals, "expected 1 results got 2")
c.Assert(called, jc.IsTrue)
}

func (s *provisionerSuite) TestProvisionerConfig(c *gc.C) {
var called bool
client := newClient(func(objType string, version int, id, request string, a, result interface{}) error {
called = true
c.Check(objType, gc.Equals, "CAASApplicationProvisioner")
c.Check(id, gc.Equals, "")
c.Assert(request, gc.Equals, "ProvisionerConfig")
c.Assert(a, gc.IsNil)
c.Assert(result, gc.FitsTypeOf, &params.CAASApplicationProvisionerConfigResult{})
*(result.(*params.CAASApplicationProvisionerConfigResult)) = params.CAASApplicationProvisionerConfigResult{
ProvisionerConfig: &params.CAASApplicationProvisionerConfig{
UnmanagedApplications: params.Entities{Entities: []params.Entity{{Tag: "application-controller"}}},
},
}
return nil
})
result, err := client.ProvisionerConfig()
c.Assert(err, jc.ErrorIsNil)
c.Assert(called, jc.IsTrue)
c.Assert(result, gc.DeepEquals, params.CAASApplicationProvisionerConfig{
UnmanagedApplications: params.Entities{Entities: []params.Entity{{Tag: "application-controller"}}},
})
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ type mockState struct {
resource *mockResources
operatorRepo string
controllerConfigWatcher *statetesting.MockNotifyWatcher
isController bool
}

func newMockState() *mockState {
Expand Down Expand Up @@ -120,6 +121,11 @@ func (st *mockState) Resources() caasapplicationprovisioner.Resources {
return st.resource
}

func (st *mockState) IsController() bool {
st.MethodCall(st, "IsController")
return st.isController
}

type mockResources struct {
caasapplicationprovisioner.Resources
resource *resources.DockerImageDetails
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"github.com/juju/juju/core/network"
"github.com/juju/juju/core/resources"
"github.com/juju/juju/core/status"
"github.com/juju/juju/environs/bootstrap"
"github.com/juju/juju/environs/config"
"github.com/juju/juju/environs/tags"
"github.com/juju/juju/resource"
Expand Down Expand Up @@ -1390,3 +1391,17 @@ func (a *API) SetProvisioningState(args params.CAASApplicationProvisioningStateA

return result, nil
}

// ProvisionerConfig returns the provisioner's configuration.
func (a *API) ProvisionerConfig() (params.CAASApplicationProvisionerConfigResult, error) {
result := params.CAASApplicationProvisionerConfigResult{
ProvisionerConfig: &params.CAASApplicationProvisionerConfig{},
}
if a.state.IsController() {
result.ProvisionerConfig.UnmanagedApplications.Entities = append(
result.ProvisionerConfig.UnmanagedApplications.Entities,
params.Entity{Tag: names.NewApplicationTag(bootstrap.ControllerApplicationName).String()},
)
}
return result, nil
}
Original file line number Diff line number Diff line change
Expand Up @@ -663,3 +663,18 @@ func (s *CAASApplicationProvisionerSuite) TestProvisioningState(c *gc.C) {

s.st.app.Stub.CheckCallNames(c, "ProvisioningState", "SetProvisioningState", "ProvisioningState")
}

func (s *CAASApplicationProvisionerSuite) TestProvisionerConfig(c *gc.C) {
result, err := s.api.ProvisionerConfig()
c.Assert(err, jc.ErrorIsNil)
c.Assert(result.Error, gc.IsNil)
c.Assert(result.ProvisionerConfig, gc.NotNil)
c.Assert(result.ProvisionerConfig.UnmanagedApplications.Entities, gc.HasLen, 0)

s.st.isController = true
result, err = s.api.ProvisionerConfig()
c.Assert(err, jc.ErrorIsNil)
c.Assert(result.Error, gc.IsNil)
c.Assert(result.ProvisionerConfig, gc.NotNil)
c.Assert(result.ProvisionerConfig.UnmanagedApplications.Entities, gc.DeepEquals, []params.Entity{{Tag: "application-controller"}})
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ type CAASApplicationProvisionerState interface {
Resources() Resources
Unit(string) (Unit, error)
WatchApplications() state.StringsWatcher
IsController() bool
}

// CAASApplicationControllerState provides the subset of controller state
Expand Down
30 changes: 30 additions & 0 deletions apiserver/facades/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -6485,6 +6485,15 @@
},
"description": "Life returns the life status of every supplied entity, where available."
},
"ProvisionerConfig": {
"type": "object",
"properties": {
"Result": {
"$ref": "#/definitions/CAASApplicationProvisionerConfigResult"
}
},
"description": "ProvisionerConfig returns the provisioner's configuration."
},
"ProvisioningInfo": {
"type": "object",
"properties": {
Expand Down Expand Up @@ -6757,6 +6766,27 @@
"images"
]
},
"CAASApplicationProvisionerConfig": {
"type": "object",
"properties": {
"unmanaged-applications": {
"$ref": "#/definitions/Entities"
}
},
"additionalProperties": false
},
"CAASApplicationProvisionerConfigResult": {
"type": "object",
"properties": {
"error": {
"$ref": "#/definitions/Error"
},
"provisioner-config": {
"$ref": "#/definitions/CAASApplicationProvisionerConfig"
}
},
"additionalProperties": false
},
"CAASApplicationProvisioningInfo": {
"type": "object",
"properties": {
Expand Down
2 changes: 1 addition & 1 deletion caas/kubernetes/provider/bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -1238,7 +1238,7 @@ func (c *controllerStack) controllerContainers(setupCmd, machineCmd, controllerI
// Write it to a file so it can be executed.
mongoStartup = strings.ReplaceAll(mongoStartup, "\n", "\\n")
makeMongoCmd := fmt.Sprintf("printf '%s'>%s", mongoStartup, mongoSh)
mongoArgs := fmt.Sprintf("%[1]s && chmod a+x %[2]s && %[2]s", makeMongoCmd, mongoSh)
mongoArgs := fmt.Sprintf("%[1]s && chmod a+x %[2]s && exec %[2]s", makeMongoCmd, mongoSh)
logger.Debugf("mongodb container args:\n%s", mongoArgs)

dbImage, err := c.pcfg.GetJujuDbOCIImagePath()
Expand Down
4 changes: 2 additions & 2 deletions caas/kubernetes/provider/bootstrap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -690,7 +690,7 @@ func (s *bootstrapSuite) TestBootstrap(c *gc.C) {
},
Args: []string{
"-c",
`printf 'args="--dbpath=/var/lib/juju/db --tlsCertificateKeyFile=/var/lib/juju/server.pem --tlsCertificateKeyFilePassword=ignored --tlsMode=requireTLS --port=1234 --journal --replSet=juju --quiet --oplogSize=1024 --auth --keyFile=/var/lib/juju/shared-secret --storageEngine=wiredTiger --bind_ip_all"\nipv6Disabled=$(sysctl net.ipv6.conf.all.disable_ipv6 -n)\nif [ $ipv6Disabled -eq 0 ]; then\n args="${args} --ipv6"\nfi\nexec mongod ${args}\n'>/root/mongo.sh && chmod a+x /root/mongo.sh && /root/mongo.sh`,
`printf 'args="--dbpath=/var/lib/juju/db --tlsCertificateKeyFile=/var/lib/juju/server.pem --tlsCertificateKeyFilePassword=ignored --tlsMode=requireTLS --port=1234 --journal --replSet=juju --quiet --oplogSize=1024 --auth --keyFile=/var/lib/juju/shared-secret --storageEngine=wiredTiger --bind_ip_all"\nipv6Disabled=$(sysctl net.ipv6.conf.all.disable_ipv6 -n)\nif [ $ipv6Disabled -eq 0 ]; then\n args="${args} --ipv6"\nfi\nexec mongod ${args}\n'>/root/mongo.sh && chmod a+x /root/mongo.sh && exec /root/mongo.sh`,
},
Ports: []core.ContainerPort{
{
Expand Down Expand Up @@ -789,7 +789,7 @@ services:
EOF
/opt/pebble run --http :38811 --verbose
exec /opt/pebble run --http :38811 --verbose
`[1:],
},
WorkingDir: "/var/lib/juju",
Expand Down
2 changes: 1 addition & 1 deletion caas/kubernetes/provider/k8s.go
Original file line number Diff line number Diff line change
Expand Up @@ -1389,7 +1389,7 @@ func getJujuInitContainerAndStorageInfo(operatorImagePath string) (container cor
jujudCmd := `
initCmd=$($JUJU_TOOLS_DIR/jujud help commands | grep caas-unit-init)
if test -n "$initCmd"; then
$JUJU_TOOLS_DIR/jujud caas-unit-init --debug --wait;
exec $JUJU_TOOLS_DIR/jujud caas-unit-init --debug --wait;
else
exit 0
fi`[1:]
Expand Down
2 changes: 1 addition & 1 deletion caas/kubernetes/provider/k8s_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8392,7 +8392,7 @@ cp /opt/jujud $JUJU_TOOLS_DIR/jujud
jujudCmd += `
initCmd=$($JUJU_TOOLS_DIR/jujud help commands | grep caas-unit-init)
if test -n "$initCmd"; then
$JUJU_TOOLS_DIR/jujud caas-unit-init --debug --wait;
exec $JUJU_TOOLS_DIR/jujud caas-unit-init --debug --wait;
else
exit 0
fi
Expand Down
2 changes: 1 addition & 1 deletion caas/kubernetes/provider/modeloperator.go
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ func modelOperatorDeployment(
volumes []core.Volume,
volumeMounts []core.VolumeMount,
) (o *apps.Deployment, err error) {
jujudCmd := fmt.Sprintf("$JUJU_TOOLS_DIR/jujud model --model-uuid=%s", modelUUID)
jujudCmd := fmt.Sprintf("exec $JUJU_TOOLS_DIR/jujud model --model-uuid=%s", modelUUID)
jujuDataDir := paths.DataDir(paths.OSUnixLike)

o = &apps.Deployment{
Expand Down
2 changes: 1 addition & 1 deletion caas/kubernetes/provider/operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -801,7 +801,7 @@ func operatorPod(
}

appTag := names.NewApplicationTag(appName)
jujudCmd := fmt.Sprintf("$JUJU_TOOLS_DIR/jujud caasoperator --application-name=%s --debug", appName)
jujudCmd := fmt.Sprintf("exec $JUJU_TOOLS_DIR/jujud caasoperator --application-name=%s --debug", appName)
jujuDataDir := paths.DataDir(paths.OSUnixLike)
mountToken := true
env := []core.EnvVar{
Expand Down
2 changes: 1 addition & 1 deletion caas/kubernetes/provider/operator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ export JUJU_TOOLS_DIR=$JUJU_DATA_DIR/tools
mkdir -p $JUJU_TOOLS_DIR
cp /opt/juju/jujud $JUJU_TOOLS_DIR/jujud
$JUJU_TOOLS_DIR/jujud caasoperator --application-name=test --debug
exec $JUJU_TOOLS_DIR/jujud caasoperator --application-name=test --debug
`[1:],
},
Env: []core.EnvVar{
Expand Down
2 changes: 1 addition & 1 deletion caas/kubernetes/provider/operator_upgrade_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ func (o *OperatorUpgraderSuite) TestOperatorUpgradeToBaseCharm(c *gc.C) {
MountPath: "/opt/juju",
}})
c.Assert(operatorSS.Spec.Template.Spec.Containers[0].Image, gc.Equals, focalCharmBase)
c.Assert(operatorSS.Spec.Template.Spec.Containers[0].Args, gc.DeepEquals, []string{"-c", "export JUJU_DATA_DIR=/var/lib/juju\nexport JUJU_TOOLS_DIR=$JUJU_DATA_DIR/tools\n\nmkdir -p $JUJU_TOOLS_DIR\ncp /opt/juju/jujud $JUJU_TOOLS_DIR/jujud\n\n$JUJU_TOOLS_DIR/jujud caasoperator --application-name=testinitss --debug\n"})
c.Assert(operatorSS.Spec.Template.Spec.Containers[0].Args, gc.DeepEquals, []string{"-c", "export JUJU_DATA_DIR=/var/lib/juju\nexport JUJU_TOOLS_DIR=$JUJU_DATA_DIR/tools\n\nmkdir -p $JUJU_TOOLS_DIR\ncp /opt/juju/jujud $JUJU_TOOLS_DIR/jujud\n\nexec $JUJU_TOOLS_DIR/jujud caasoperator --application-name=testinitss --debug\n"})
c.Assert(operatorSS.Spec.Template.Spec.Containers[0].VolumeMounts, gc.DeepEquals, []v1.VolumeMount{{
Name: "juju-bins",
MountPath: "/opt/juju",
Expand Down
2 changes: 1 addition & 1 deletion caas/kubernetes/provider/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ func patchOperatorToCharmBase(ss *apps.StatefulSet, appName string, imagePath st
continue
}

jujudCmd := fmt.Sprintf("$JUJU_TOOLS_DIR/jujud caasoperator --application-name=%s --debug", appName)
jujudCmd := fmt.Sprintf("exec $JUJU_TOOLS_DIR/jujud caasoperator --application-name=%s --debug", appName)
jujuDataDir := paths.DataDir(paths.OSUnixLike)
container.Image = baseImagePath
container.Args = []string{
Expand Down
2 changes: 1 addition & 1 deletion caas/scripts.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,6 @@ cat > /var/lib/pebble/default/layers/001-jujud.yaml <<EOF
%[3]s
EOF
/opt/pebble run --http :%[4]s --verbose
exec /opt/pebble run --http :%[4]s --verbose
`[1:]
)
2 changes: 1 addition & 1 deletion cmd/modelcmd/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ func (c *CommandBase) NewAPIRootWithDialOpts(
if proxyerrors.IsProxyConnectError(err) {
logger.Debugf("proxy connection error: %v", err)
if proxyerrors.ProxyType(err) == k8sproxy.ProxierTypeKey {
return nil, errors.New("cannot connect to k8s api server; try running 'juju update-k8s --client <k8s cloud name>'")
return nil, errors.Annotate(err, "cannot connect to k8s api server; try running 'juju update-k8s --client <k8s cloud name>'")
}
return nil, errors.Annotate(err, "cannot connect to api server proxy")
}
Expand Down
12 changes: 12 additions & 0 deletions rpc/params/caas.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,3 +185,15 @@ type CAASApplicationProvisioningStateArg struct {
Application Entity `json:"application"`
ProvisioningState CAASApplicationProvisioningState `json:"provisioning-state"`
}

// CAASApplicationProvisionerConfig holds the configuration for the caasapplicationprovisioner worker.
type CAASApplicationProvisionerConfig struct {
UnmanagedApplications Entities `json:"unmanaged-applications,omitempty"`
}

// CAASApplicationProvisionerConfigResult is the result of getting the caasapplicationprovisioner worker's
// configuration for the current model.
type CAASApplicationProvisionerConfigResult struct {
ProvisionerConfig *CAASApplicationProvisionerConfig `json:"provisioner-config,omitempty"`
Error *Error `json:"error,omitempty"`
}
15 changes: 15 additions & 0 deletions worker/caasapplicationprovisioner/mocks/facade_mock.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 18 additions & 0 deletions worker/caasapplicationprovisioner/worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"time"

"github.com/juju/clock"
"github.com/juju/collections/set"
"github.com/juju/errors"
"github.com/juju/names/v4"
"github.com/juju/worker/v3"
Expand Down Expand Up @@ -64,6 +65,7 @@ type CAASProvisionerFacade interface {
DestroyUnits(unitNames []string) error
ProvisioningState(string) (*params.CAASApplicationProvisioningState, error)
SetProvisioningState(string, params.CAASApplicationProvisioningState) error
ProvisionerConfig() (params.CAASApplicationProvisionerConfig, error)
}

// CAASBroker exposes CAAS broker functionality to a worker.
Expand Down Expand Up @@ -159,6 +161,19 @@ func (p *provisioner) loop() error {
return errors.Trace(err)
}

config, err := p.facade.ProvisionerConfig()
if err != nil {
return errors.Trace(err)
}
unmanagedApps := set.NewStrings()
for _, v := range config.UnmanagedApplications.Entities {
app, err := names.ParseApplicationTag(v.Tag)
if err != nil {
return errors.Trace(err)
}
unmanagedApps.Add(app.Name)
}

for {
select {
case <-p.catacomb.Dying():
Expand All @@ -168,6 +183,9 @@ func (p *provisioner) loop() error {
return errors.New("app watcher closed channel")
}
for _, appName := range apps {
if unmanagedApps.Contains(appName) {
continue
}
_, err := p.facade.Life(appName)
if err != nil && !errors.IsNotFound(err) {
return errors.Trace(err)
Expand Down
Loading

0 comments on commit eae516e

Please sign in to comment.