diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index c3256cf3e87..a8863d5a295 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -35,7 +35,7 @@ jobs: steps: - name: "Checkout" - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: "Set up Go" uses: actions/setup-go@v5 diff --git a/.github/workflows/client-tests.yml b/.github/workflows/client-tests.yml index 76b82349b85..6ffdedc4736 100644 --- a/.github/workflows/client-tests.yml +++ b/.github/workflows/client-tests.yml @@ -29,7 +29,7 @@ jobs: steps: - name: "Checkout" - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: "Set up Go" uses: actions/setup-go@v5 diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 9a6ea142995..d4a60dffa25 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -17,7 +17,7 @@ jobs: steps: - name: Checkout code - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Set up Go uses: actions/setup-go@v5 diff --git a/.github/workflows/gen.yml b/.github/workflows/gen.yml index 8007eb16e24..77ab76adfcf 100644 --- a/.github/workflows/gen.yml +++ b/.github/workflows/gen.yml @@ -23,7 +23,7 @@ jobs: steps: - name: "Checkout" - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: "Set up Go" uses: actions/setup-go@v5 diff --git a/.github/workflows/license.yml b/.github/workflows/license.yml index 663b90dd3f6..278176d369e 100644 --- a/.github/workflows/license.yml +++ b/.github/workflows/license.yml @@ -20,7 +20,7 @@ jobs: if: github.event.pull_request.draft == false steps: - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Set up Go uses: actions/setup-go@v5 with: diff --git a/.github/workflows/merge.yml b/.github/workflows/merge.yml index 7502143abe8..14091584a8a 100644 --- a/.github/workflows/merge.yml +++ b/.github/workflows/merge.yml @@ -26,7 +26,7 @@ jobs: echo "target=$TARGET_BRANCH" >> "$GITHUB_OUTPUT" - name: Checkout repository - uses: actions/checkout@v3 + uses: actions/checkout@v4 with: fetch-depth: 0 ref: ${{ steps.branch.outputs.source }} diff --git a/.github/workflows/microk8s-tests.yml b/.github/workflows/microk8s-tests.yml index 82cb947d0f6..af6c32eb1fe 100644 --- a/.github/workflows/microk8s-tests.yml +++ b/.github/workflows/microk8s-tests.yml @@ -23,7 +23,7 @@ jobs: steps: - name: Checking out repo - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Set up Go uses: actions/setup-go@v5 diff --git a/.github/workflows/migrate.yml b/.github/workflows/migrate.yml index ab582fef6bb..5a9b838a243 100644 --- a/.github/workflows/migrate.yml +++ b/.github/workflows/migrate.yml @@ -33,7 +33,7 @@ jobs: steps: - name: Checkout code - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Set up Go uses: actions/setup-go@v5 diff --git a/.github/workflows/smoke.yml b/.github/workflows/smoke.yml index d1eb8e84b17..bf6c3fcbd9e 100644 --- a/.github/workflows/smoke.yml +++ b/.github/workflows/smoke.yml @@ -29,7 +29,7 @@ jobs: sudo DEBIAN_FRONTEND=noninteractive apt install -y expect - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Setup LXD if: matrix.cloud == 'localhost' diff --git a/.github/workflows/snap.yml b/.github/workflows/snap.yml index bf5ca4a2058..0091a6cc041 100644 --- a/.github/workflows/snap.yml +++ b/.github/workflows/snap.yml @@ -33,7 +33,7 @@ jobs: echo "/snap/bin" >> $GITHUB_PATH - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Setup LXD uses: canonical/setup-lxd@4e959f8e0d9c5feb27d44c5e4d9a330a782edee0 diff --git a/.github/workflows/static-analysis.yml b/.github/workflows/static-analysis.yml index fa8cddb375a..17fdc25799e 100644 --- a/.github/workflows/static-analysis.yml +++ b/.github/workflows/static-analysis.yml @@ -18,7 +18,7 @@ jobs: if: github.event.pull_request.draft == false steps: - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Determine which tests to run uses: dorny/paths-filter@v3 @@ -101,7 +101,7 @@ jobs: if: github.event.pull_request.draft == false steps: - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Check if there is anything to test uses: dorny/paths-filter@v3 @@ -160,10 +160,10 @@ jobs: binary: ["jujud", "juju"] steps: - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Set up Go - uses: actions/setup-go@v4 + uses: actions/setup-go@v5 with: go-version-file: 'go.mod' cache: true diff --git a/.github/workflows/terraform-smoke.yml b/.github/workflows/terraform-smoke.yml index 6576336a7d5..096292e95af 100644 --- a/.github/workflows/terraform-smoke.yml +++ b/.github/workflows/terraform-smoke.yml @@ -29,7 +29,7 @@ jobs: sudo DEBIAN_FRONTEND=noninteractive apt install -y expect - name: Checkout juju - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Setup LXD uses: canonical/setup-lxd@4e959f8e0d9c5feb27d44c5e4d9a330a782edee0 @@ -61,7 +61,7 @@ jobs: juju version - name: Find terraform provider for juju latest release - uses: actions/checkout@v3 + uses: actions/checkout@v4 with: repository: 'juju/terraform-provider-juju' #path: terraform-provider-juju diff --git a/.github/workflows/upgrade.yml b/.github/workflows/upgrade.yml index 7e17ead0433..3727c1c596d 100644 --- a/.github/workflows/upgrade.yml +++ b/.github/workflows/upgrade.yml @@ -29,7 +29,7 @@ jobs: channel: ${{ steps.check.outputs.channel }} steps: - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Check id: check run: | @@ -62,7 +62,7 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Setup LXD if: matrix.cloud == 'localhost' diff --git a/PULL_REQUEST_TEMPLATE.md b/PULL_REQUEST_TEMPLATE.md index ea38f35d808..d61bb3a9e7e 100644 --- a/PULL_REQUEST_TEMPLATE.md +++ b/PULL_REQUEST_TEMPLATE.md @@ -1,3 +1,10 @@ + + ## Checklist diff --git a/provider/openstack/firewaller.go b/provider/openstack/firewaller.go index 60656a22877..956ad451b67 100644 --- a/provider/openstack/firewaller.go +++ b/provider/openstack/firewaller.go @@ -73,6 +73,13 @@ type Firewaller interface { // If the model security group doesn't exist, return a NotFound error ModelIngressRules(ctx context.ProviderCallContext) (firewall.IngressRules, error) + // DeleteMachineGroup delete's the security group specific to the provided machine. + // When in 'instance' firewall mode, each instance in a model is assigned its own + // security group, with a lifecycle matching that of the instance itself. + // In 'global' mode, all security groups are model scoped, and have lifecycles + // matching the model, so this method will remove no groups. + DeleteMachineGroup(ctx context.ProviderCallContext, machineId string) error + // DeleteAllModelGroups deletes all security groups for the // model. DeleteAllModelGroups(ctx context.ProviderCallContext) error @@ -463,6 +470,10 @@ func (c *neutronFirewaller) DeleteAllModelGroups(ctx context.ProviderCallContext return deleteSecurityGroupsMatchingName(ctx, c.deleteSecurityGroups, c.jujuGroupPrefixRegexp()) } +func (c *neutronFirewaller) DeleteMachineGroup(ctx context.ProviderCallContext, machineID string) error { + return deleteSecurityGroupsMatchingName(ctx, c.deleteSecurityGroups, c.machineGroupRegexp(machineID)) +} + // UpdateGroupController implements Firewaller interface. func (c *neutronFirewaller) UpdateGroupController(ctx context.ProviderCallContext, controllerUUID string) error { neutronClient := c.environ.neutron() diff --git a/provider/openstack/local_test.go b/provider/openstack/local_test.go index d08a66cf3f4..d284713ccf0 100644 --- a/provider/openstack/local_test.go +++ b/provider/openstack/local_test.go @@ -849,7 +849,7 @@ func (s *localServerSuite) TestStartInstanceWaitForActiveDetails(c *gc.C) { c.Assert(insts, gc.HasLen, 0, gc.Commentf("expected launched instance to be terminated if stuck in BUILD state")) } -func (s *localServerSuite) TestStartInstanceDeletesSecurityGroupsOnInstanceCreateFailure(c *gc.C) { +func (s *localServerSuite) TestStartInstanceDeletesMachineSecurityGroupOnInstanceCreateFailure(c *gc.C) { env := s.openEnviron(c, coretesting.Attrs{"firewall-mode": config.FwInstance}) // Force an error in waitForActiveServerDetails @@ -864,7 +864,10 @@ func (s *localServerSuite) TestStartInstanceDeletesSecurityGroupsOnInstanceCreat c.Check(inst, gc.IsNil) c.Assert(err, gc.NotNil) - assertSecurityGroups(c, env, []string{"default"}) + assertSecurityGroups(c, env, []string{ + fmt.Sprintf("juju-%s-%s", coretesting.ControllerTag.Id(), coretesting.ModelTag.Id()), + "default", + }) } func (s *localServerSuite) TestStartInstanceDeletesSecurityGroupsOnFailure(c *gc.C) { @@ -881,7 +884,10 @@ func (s *localServerSuite) TestStartInstanceDeletesSecurityGroupsOnFailure(c *gc _, _, _, err := testing.StartInstance(env, s.callCtx, s.ControllerUUID, "100") c.Assert(err, gc.NotNil) - assertSecurityGroups(c, env, []string{"default"}) + assertSecurityGroups(c, env, []string{ + fmt.Sprintf("juju-%s-%s", coretesting.ControllerTag.Id(), coretesting.ModelTag.Id()), + "default", + }) } func assertSecurityGroups(c *gc.C, env environs.Environ, expected []string) { @@ -3062,8 +3068,8 @@ func (s *localServerSuite) TestUpdateGroupController(c *gc.C) { c.Assert(err, jc.ErrorIsNil) groupNamesBefore := set.NewStrings(groupNames...) c.Assert(groupNamesBefore, gc.DeepEquals, set.NewStrings( - "juju-deadbeef-1bad-500d-9000-4b1d0d06f00d-deadbeef-0bad-400d-8000-4b1d0d06f00d", - "juju-deadbeef-1bad-500d-9000-4b1d0d06f00d-deadbeef-0bad-400d-8000-4b1d0d06f00d-0", + fmt.Sprintf("juju-%s-%s", coretesting.ControllerTag.Id(), coretesting.ModelTag.Id()), + fmt.Sprintf("juju-%s-%s-0", coretesting.ControllerTag.Id(), coretesting.ModelTag.Id()), )) firewaller := openstack.GetFirewaller(s.env) @@ -3074,8 +3080,8 @@ func (s *localServerSuite) TestUpdateGroupController(c *gc.C) { c.Assert(err, jc.ErrorIsNil) groupNamesAfter := set.NewStrings(groupNames...) c.Assert(groupNamesAfter, gc.DeepEquals, set.NewStrings( - "juju-aabbccdd-eeee-ffff-0000-0123456789ab-deadbeef-0bad-400d-8000-4b1d0d06f00d", - "juju-aabbccdd-eeee-ffff-0000-0123456789ab-deadbeef-0bad-400d-8000-4b1d0d06f00d-0", + fmt.Sprintf("juju-aabbccdd-eeee-ffff-0000-0123456789ab-%s", coretesting.ModelTag.Id()), + fmt.Sprintf("juju-aabbccdd-eeee-ffff-0000-0123456789ab-%s-0", coretesting.ModelTag.Id()), )) } diff --git a/provider/openstack/provider.go b/provider/openstack/provider.go index 169a2005e9b..cb6fb136692 100644 --- a/provider/openstack/provider.go +++ b/provider/openstack/provider.go @@ -1273,7 +1273,7 @@ func (e *Environ) startInstance( server, err := tryStartNovaInstance(shortAttempt, e.nova(), opts) if err != nil || server == nil { // Attempt to clean up any security groups we created. - if err := e.cleanupGroups(ctx, e.nova(), novaGroupNames); err != nil { + if err := e.firewaller.DeleteMachineGroup(ctx, args.InstanceConfig.MachineId); err != nil { // If we failed to clean up the security groups, we need the user // to manually clean them up. logger.Errorf("cannot cleanup security groups: %v", err) @@ -1351,19 +1351,6 @@ func (e *Environ) startInstance( }, nil } -// Clean up any groups that we have created if we fail to start the instance. -func (e *Environ) cleanupGroups( - ctx context.ProviderCallContext, - client *nova.Client, - groups []nova.SecurityGroupName, -) error { - names := make([]string, len(groups)) - for i, group := range groups { - names[i] = group.Name - } - return e.firewaller.DeleteGroups(ctx, names...) -} - func (e *Environ) userFriendlyInvalidNetworkError(err error) error { msg := fmt.Sprintf("%s\n\t%s\n\t%s", err.Error(), "This error was caused by juju attempting to create an OpenStack instance with no network defined.",