Skip to content

Commit

Permalink
Merge branch '3.5' into 3.5-into-main
Browse files Browse the repository at this point in the history
Conflicts:
- apiserver/facades/client/application/deployrepository.go
- apiserver/facades/client/application/deployrepository_test.go
- internal/worker/uniter/container/workload_test.go
  • Loading branch information
jack-w-shaw committed Apr 16, 2024
2 parents 63eb06d + a753888 commit bce33b0
Show file tree
Hide file tree
Showing 16 changed files with 45 additions and 205 deletions.
9 changes: 7 additions & 2 deletions apiserver/facades/client/application/deployrepository.go
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,9 @@ type caasDeployFromRepositoryValidator struct {
// - Check the charm's min version against the caasVersion
func (v caasDeployFromRepositoryValidator) ValidateArg(ctx context.Context, arg params.DeployFromRepositoryArg) (deployTemplate, []error) {
dt, errs := v.validator.validate(ctx, arg)

if len(errs) > 0 {
return dt, errs
}
if charm.MetaFormat(dt.charm) == charm.FormatV1 {
errs = append(errs, errors.NotSupportedf("deploying format v1 charm %q", arg.CharmName))
}
Expand All @@ -558,11 +560,14 @@ type iaasDeployFromRepositoryValidator struct {
validator *deployFromRepositoryValidator
}

// ValidateArg validates DeployFromRepositoryArg from a iaas perspective.
// ValidateArg validates DeployFromRepositoryArg from an iaas perspective.
// First checking the common validation, then any validation specific to
// iaas charms.
func (v iaasDeployFromRepositoryValidator) ValidateArg(ctx context.Context, arg params.DeployFromRepositoryArg) (deployTemplate, []error) {
dt, errs := v.validator.validate(ctx, arg)
if len(errs) > 0 {
return dt, errs
}
attachStorage, attachStorageErrs := validateAndParseAttachStorage(arg.AttachStorage, dt.numUnits)
if len(attachStorageErrs) > 0 {
errs = append(errs, attachStorageErrs...)
Expand Down
29 changes: 29 additions & 0 deletions apiserver/facades/client/application/deployrepository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package application

import (
"context"
"fmt"
"reflect"

"github.com/juju/charm/v13"
Expand Down Expand Up @@ -1020,6 +1021,34 @@ func (s *validatorSuite) TestCaasDeployFromRepositoryValidator(c *gc.C) {
})
}

func (s *validatorSuite) TestIaaSDeployFromRepositoryFailResolveCharm(c *gc.C) {
defer s.setupMocks(c).Finish()
s.expectSimpleValidate()
s.repo.EXPECT().ResolveForDeploy(gomock.Any(), gomock.Any()).Return(corecharm.ResolvedDataForDeploy{}, fmt.Errorf("fail resolve"))
s.model.EXPECT().UUID().Return("")

arg := params.DeployFromRepositoryArg{
CharmName: "testcharm",
}

_, errs := s.iaasDeployFromRepositoryValidator().ValidateArg(context.Background(), arg)
c.Assert(errs, gc.HasLen, 1)
}

func (s *validatorSuite) TestCaaSDeployFromRepositoryFailResolveCharm(c *gc.C) {
defer s.setupMocks(c).Finish()
s.expectSimpleValidate()
s.repo.EXPECT().ResolveForDeploy(gomock.Any(), gomock.Any()).Return(corecharm.ResolvedDataForDeploy{}, fmt.Errorf("fail resolve"))
s.model.EXPECT().UUID().Return("")

arg := params.DeployFromRepositoryArg{
CharmName: "testcharm",
}

_, errs := s.caasDeployFromRepositoryValidator(c).ValidateArg(context.Background(), arg)
c.Assert(errs, gc.HasLen, 1)
}

func getResolvedData(resultURL *charm.URL, resolvedOrigin corecharm.Origin) corecharm.ResolvedDataForDeploy {
expMeta := &charm.Meta{
Name: "test-charm",
Expand Down
4 changes: 4 additions & 0 deletions internal/cloudconfig/userdatacfg_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,10 @@ func (w *unixConfigure) ConfigureJuju() error {
// To keep postruncmd at the end of any runcmd's that juju adds,
// this block must stay at the top.
if postruncmds, ok := w.icfg.CloudInitUserData["postruncmd"].([]interface{}); ok {

// revert the `set -xe` shell flag which was set after preruncmd
// LP: #1978454
w.conf.AddRunCmd("set +xe")
cmds := make([]string, len(postruncmds))
for i, v := range postruncmds {
cmd, err := runCmdToString(v)
Expand Down
9 changes: 0 additions & 9 deletions internal/worker/uniter/container/workload.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ const (
// ReadyEvent is triggered when the container/pebble starts up.
ReadyEvent WorkloadEventType = iota
CustomNoticeEvent
ChangeUpdatedEvent
)

// WorkloadEvent contains information about the event type and data associated with
Expand Down Expand Up @@ -200,14 +199,6 @@ func (r *workloadHookResolver) NextOp(
NoticeType: evt.NoticeType,
NoticeKey: evt.NoticeKey,
})
case ChangeUpdatedEvent:
op, err = opFactory.NewRunHook(hook.Info{
Kind: hooks.PebbleChangeUpdated,
WorkloadName: evt.WorkloadName,
NoticeID: evt.NoticeID,
NoticeType: evt.NoticeType,
NoticeKey: evt.NoticeKey,
})
case ReadyEvent:
op, err = opFactory.NewRunHook(hook.Info{
Kind: hooks.PebbleReady,
Expand Down
43 changes: 0 additions & 43 deletions internal/worker/uniter/container/workload_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,46 +133,3 @@ func (s *workloadSuite) TestWorkloadCustomNoticeHook(c *gc.C) {
NoticeKey: "example.com/foo",
})
}

func (s *workloadSuite) TestWorkloadChangeUpdatedHook(c *gc.C) {
events := container.NewWorkloadEvents()
expectedErr := errors.Errorf("expected error")
handler := func(err error) {
c.Assert(err, gc.Equals, expectedErr)
}
containerResolver := container.NewWorkloadHookResolver(
loggo.GetLogger("test"),
events,
events.RemoveWorkloadEvent)
localState := resolver.LocalState{
State: operation.State{
Kind: operation.Continue,
Step: operation.Pending,
},
}
remoteState := remotestate.Snapshot{
WorkloadEvents: []string{
events.AddWorkloadEvent(container.WorkloadEvent{
Type: container.ChangeUpdatedEvent,
WorkloadName: "test",
NoticeID: "123",
NoticeType: "change-update",
NoticeKey: "42",
}, handler),
},
}
opFactory := &mockOperations{}
op, err := containerResolver.NextOp(context.Background(), localState, remoteState, opFactory)
c.Assert(err, jc.ErrorIsNil)
c.Assert(op, gc.NotNil)
op = operation.Unwrap(op)
hookOp, ok := op.(*mockRunHookOp)
c.Assert(ok, jc.IsTrue)
c.Assert(hookOp.hookInfo, gc.DeepEquals, hook.Info{
Kind: "pebble-change-updated",
WorkloadName: "test",
NoticeID: "123",
NoticeType: "change-update",
NoticeKey: "42",
})
}
2 changes: 1 addition & 1 deletion internal/worker/uniter/hook/hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func (hi Info) Validate() error {
return errors.Errorf("%q hook has a remote unit but no application", hi.Kind)
}
return nil
case hooks.PebbleCustomNotice, hooks.PebbleChangeUpdated:
case hooks.PebbleCustomNotice:
if hi.WorkloadName == "" {
return errors.Errorf("%q hook requires a workload name", hi.Kind)
}
Expand Down
6 changes: 0 additions & 6 deletions internal/worker/uniter/hook/hook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,6 @@ var validateTests = []struct {
}, {
hook.Info{Kind: hooks.PebbleCustomNotice, WorkloadName: "test"},
`"pebble-custom-notice" hook requires a notice ID, type, and key`,
}, {
hook.Info{Kind: hooks.PebbleChangeUpdated},
`"pebble-change-updated" hook requires a workload name`,
}, {
hook.Info{Kind: hooks.PebbleChangeUpdated, WorkloadName: "test"},
`"pebble-change-updated" hook requires a notice ID, type, and key`,
}, {
hook.Info{Kind: hooks.PreSeriesUpgrade},
`"pre-series-upgrade" hook requires a target base`,
Expand Down
2 changes: 0 additions & 2 deletions internal/worker/uniter/pebblenotices.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,6 @@ func (n *pebbleNoticer) processNotice(containerName string, notice *client.Notic
switch notice.Type {
case client.CustomNotice:
eventType = container.CustomNoticeEvent
case client.ChangeUpdateNotice:
eventType = container.ChangeUpdatedEvent
default:
n.logger.Debugf("container %q: ignoring %s notice", containerName, notice.Type)
return nil
Expand Down
19 changes: 0 additions & 19 deletions internal/worker/uniter/pebblenotices_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,25 +113,6 @@ func (s *pebbleNoticerSuite) TestWaitNotices(c *gc.C) {
})
}

func (s *pebbleNoticerSuite) TestChangeUpdate(c *gc.C) {
s.setUpWorker(c, []string{"c1"})
defer workertest.CleanKill(c, s.worker)

s.clients["c1"].AddNotice(c, &client.Notice{
ID: "1",
Type: "change-update",
Key: "42",
LastRepeated: time.Now(),
})
s.waitWorkloadEvent(c, container.WorkloadEvent{
Type: container.ChangeUpdatedEvent,
WorkloadName: "c1",
NoticeID: "1",
NoticeType: "change-update",
NoticeKey: "42",
})
}

func (s *pebbleNoticerSuite) TestWaitNoticesError(c *gc.C) {
s.setUpWorker(c, []string{"c1"})
defer workertest.CleanKill(c, s.worker)
Expand Down
2 changes: 1 addition & 1 deletion internal/worker/uniter/runner/context/contextfactory.go
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ func (f *contextFactory) HookContext(stdCtx context.Context, hookInfo hook.Info)
ctx.workloadName = hookInfo.WorkloadName
hookName = fmt.Sprintf("%s-%s", hookInfo.WorkloadName, hookName)
switch hookInfo.Kind {
case hooks.PebbleCustomNotice, hooks.PebbleChangeUpdated:
case hooks.PebbleCustomNotice:
ctx.noticeID = hookInfo.NoticeID
ctx.noticeType = hookInfo.NoticeType
ctx.noticeKey = hookInfo.NoticeKey
Expand Down
2 changes: 1 addition & 1 deletion scripts/discourse-sync/requirements.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
certifi==2023.7.22
charset-normalizer==3.1.0
idna==3.4
idna==3.7
pydiscourse==1.3.0
PyYAML==6.0
requests==2.31.0
Expand Down
31 changes: 0 additions & 31 deletions testcharms/charms/pebble-notices/src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,24 +16,6 @@ def __init__(self, *args):
self.framework.observe(self.on["redis"].pebble_ready, self._on_pebble_ready)
self.framework.observe(self.on["redis"].pebble_custom_notice, self._on_custom_notice)

# TODO(benhoyt): update to use pebble_change_updated once ops supports that:
# https://github.com/canonical/operator/pull/1170
import os
import pathlib

dispatch_path = pathlib.Path(os.environ.get("JUJU_DISPATCH_PATH", ""))
event_name = dispatch_path.name.replace("-", "_")
logger.info(f"__init__: path={dispatch_path} event={event_name}")
if event_name == "redis_pebble_change_updated":
event = ops.PebbleNoticeEvent(
None,
self.unit.get_container(os.environ["JUJU_WORKLOAD_NAME"]),
os.environ["JUJU_NOTICE_ID"],
os.environ["JUJU_NOTICE_TYPE"],
os.environ["JUJU_NOTICE_KEY"],
)
self._on_change_updated(event)

def _on_pebble_ready(self, event):
self.unit.status = ops.ActiveStatus()

Expand All @@ -46,19 +28,6 @@ def _on_custom_notice(self, event):
# Don't include the (arbitrary) ID in the status message
self.unit.status = ops.MaintenanceStatus(f"notice type={notice_type} key={notice_key}")

def _on_change_updated(self, event):
notice_id = event.notice.id
notice_type = (
event.notice.type if isinstance(event.notice.type, str) else event.notice.type.value
)
notice_key = event.notice.key
logger.info(f"_on_change_updated: id={notice_id} type={notice_type} key={notice_key}")

change = event.workload.pebble.get_change(notice_key)
self.unit.status = ops.MaintenanceStatus(
f"notice type={notice_type} kind={change.kind} status={change.status}"
)


if __name__ == "__main__":
ops.main(PebbleNoticesCharm)
68 changes: 0 additions & 68 deletions testcharms/charms/pebble-notices/tests/unit/test_charm.py
Original file line number Diff line number Diff line change
@@ -1,18 +1,15 @@
import unittest
import unittest.mock

import ops
import ops.testing
from charm import PebbleNoticesCharm
from ops import pebble


class TestCharm(unittest.TestCase):
def setUp(self):
self.harness = ops.testing.Harness(PebbleNoticesCharm)
self.addCleanup(self.harness.cleanup)
self.harness.begin()
self._next_notice_id = 1

def test_pebble_ready(self):
self.harness.container_pebble_ready("redis")
Expand All @@ -30,68 +27,3 @@ def test_custom_notice(self):
self.harness.model.unit.status,
ops.MaintenanceStatus("notice type=custom key=ubuntu.com/bar/buzz"),
)

@unittest.mock.patch("ops.testing._TestingPebbleClient.get_change")
def test_change_updated(self, mock_get_change):
# TODO(benhoyt): update to use pebble_change_updated once ops supports that:
# https://github.com/canonical/operator/pull/1170

import os

os.environ["JUJU_DISPATCH_PATH"] = "hooks/redis-pebble-change-updated"
self.addCleanup(os.environ.__delitem__, "JUJU_DISPATCH_PATH")
self.addCleanup(os.environ.__delitem__, "JUJU_NOTICE_ID")
self.addCleanup(os.environ.__delitem__, "JUJU_NOTICE_TYPE")
self.addCleanup(os.environ.__delitem__, "JUJU_NOTICE_KEY")

mock_get_change.return_value = pebble.Change.from_dict(
{
"id": "1",
"kind": "exec",
"summary": "",
"status": "Doing",
"ready": False,
"spawn-time": "2021-01-28T14:37:02.247202105+13:00",
}
)
self._pebble_notify_change_updated("redis", "123")
self.assertEqual(
self.harness.model.unit.status,
ops.MaintenanceStatus("notice type=change-update kind=exec status=Doing"),
)
mock_get_change.assert_called_once_with("123")

mock_get_change.reset_mock()
mock_get_change.return_value = pebble.Change.from_dict(
{
"id": "2",
"kind": "changeroo",
"summary": "",
"status": "Done",
"ready": True,
"spawn-time": "2024-01-28T14:37:02.247202105+13:00",
"ready-time": "2024-01-28T14:37:04.291517768+13:00",
}
)
self._pebble_notify_change_updated("redis", "42")
self.assertEqual(
self.harness.model.unit.status,
ops.MaintenanceStatus("notice type=change-update kind=changeroo status=Done"),
)
mock_get_change.assert_called_once_with("42")

def _pebble_notify_change_updated(self, container_name, notice_key):
import os

os.environ["JUJU_NOTICE_ID"] = notice_id = str(self._next_notice_id)
self._next_notice_id += 1
os.environ["JUJU_NOTICE_TYPE"] = notice_type = "change-update"
os.environ["JUJU_NOTICE_KEY"] = notice_key
event = ops.PebbleNoticeEvent(
None,
self.harness.model.unit.get_container(container_name),
notice_id,
notice_type,
notice_key,
)
self.harness.charm._on_change_updated(event)
2 changes: 2 additions & 0 deletions tests/includes/aws_ami_creation.sh
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,9 @@ run_cleanup_ami() {
if [[ -f "${TEST_DIR}/ec2-amis" ]]; then
echo "====> Cleaning up EC2 AMIs"
while read -r ec2_ami; do
snapshot_ids=$(aws ec2 describe-images --image-ids="${ec2_ami}" | jq -r ".Images[0].BlockDeviceMappings | .[] .Ebs.SnapshotId | select(. != null)")
aws ec2 deregister-image --image-id="${ec2_ami}" >>"${TEST_DIR}/aws_cleanup"
echo ${snapshot_ids} | xargs -L 1 aws ec2 delete-snapshot --snapshot-id >>"${TEST_DIR}/aws_cleanup"
done <"${TEST_DIR}/ec2-amis"
fi

Expand Down
21 changes: 0 additions & 21 deletions tests/suites/sidecar/sidecar.sh
Original file line number Diff line number Diff line change
Expand Up @@ -89,24 +89,3 @@ test_pebble_notices() {
# Clean up model
destroy_model "${model_name}"
}

test_pebble_change_updated() {
echo

# Ensure that a valid Juju controller exists
model_name="controller-model-sidecar"
file="${TEST_DIR}/test-${model_name}.log"
ensure "${model_name}" "${file}"

# Deploy Pebble Notices test application
juju deploy juju-qa-pebble-notices
wait_for "active" '.applications["juju-qa-pebble-notices"].units["juju-qa-pebble-notices/0"]["workload-status"].current'

# Check that charm is responding correctly to a change-update notice
juju ssh --container redis juju-qa-pebble-notices/0 /charm/bin/pebble exec -- echo foo
wait_for "maintenance" '.applications["juju-qa-pebble-notices"].units["juju-qa-pebble-notices/0"]["workload-status"].current'
wait_for "notice type=change-update kind=exec status=Done" '.applications["juju-qa-pebble-notices"].units["juju-qa-pebble-notices/0"]["workload-status"].message'

# Clean up model
destroy_model "${model_name}"
}
Loading

0 comments on commit bce33b0

Please sign in to comment.