[JUJU-2847] Open port per unit4sidecar #15225

ycliuhw · 2023-02-24T06:39:54Z

This PR changes the application (vs machine) level opened ports to be grouped per unit for sidecar applications:

Allow open-port to happen from any unit, and Juju would open the ports (in the K8s service definition) that are the union of all ports that any unit has opened. So if unit/1 opened port 8080 and unit/2 opened port 4000, we'd tell K8s to open both. This would be unusual but supported.
Kind of the opposite for close-port: Juju would only close the port in the K8s service definition once all units had asked for that port to be closed.
Remove the check for is-leader.
Add model upgrades steps and model migration strategy to deal with the port ranges changed from application scope (3.1.0) to unit scope.

Checklist

Code style: imports ordered, good names, simple structure, etc
Comments saying why design decisions were made
Go unit tests, with comments saying what you're testing
~~Integration tests, with comments saying what you're testing~~
~~doc.go added or updated in changed packages~~

QA steps

juju deploy prometheus-k8s --trust
Located charm "prometheus-k8s" in charm-hub, revision 103
Deploying "prometheus-k8s" from charm-hub charm "prometheus-k8s", revision 103 in channel stable on ubuntu@20.04/stable

juju scale-application prometheus-k8s 2

juju exec --unit prometheus-k8s/0 -- open-port 3000

juju exec --unit prometheus-k8s/1 -- open-port 3000

juju exec --unit prometheus-k8s/1 -- open-port 3001

juju exec --unit prometheus-k8s/0 -- opened-ports
3000/tcp

juju exec --unit prometheus-k8s/1 -- opened-ports
3000/tcp
3001/tcp

mkubectl -nt1 get service/prometheus-k8s -o json | jq '.spec.ports'
[
  {
    "name": "prometheus-k8s",
    "port": 9090,
    "protocol": "TCP",
    "targetPort": 9090
  },
  {
    "name": "juju-3000-tcp",
    "port": 3000,
    "protocol": "TCP",
    "targetPort": 3000
  },
  {
    "name": "juju-3001-tcp",
    "port": 3001,
    "protocol": "TCP",
    "targetPort": 3001
  }
]

juju exec --unit prometheus-k8s/1 -- close-port 3001

mkubectl -nt1 get service/prometheus-k8s -o json | jq '.spec.ports'
[
  {
    "name": "prometheus-k8s",
    "port": 9090,
    "protocol": "TCP",
    "targetPort": 9090
  },
  {
    "name": "juju-3000-tcp",
    "port": 3000,
    "protocol": "TCP",
    "targetPort": 3000
  }
]

juju exec --unit prometheus-k8s/1 -- close-port 3000

mkubectl -nt1 get service/prometheus-k8s -o json | jq '.spec.ports'
[
  {
    "name": "prometheus-k8s",
    "port": 9090,
    "protocol": "TCP",
    "targetPort": 9090
  },
  {
    "name": "juju-3000-tcp",
    "port": 3000,
    "protocol": "TCP",
    "targetPort": 3000
  }
]

juju exec --unit prometheus-k8s/0 -- close-port 3000

mkubectl -nt1 get service/prometheus-k8s -o json | jq '.spec.ports'
[
  {
    "name": "prometheus-k8s",
    "port": 9090,
    "protocol": "TCP",
    "targetPort": 9090
  }
]

upgrade controller from 3.1.0 -> 3.1.1 (application opened ports should be copied across all the units.)

juju status --relations --color --storage -m k1:t1
Model  Controller  Cloud/Region        Version  SLA          Timestamp
t1     k1          microk8s/localhost  3.1.0    unsupported  21:54:23+11:00

App             Version  Status  Scale  Charm           Channel  Rev  Address        Exposed  Message
prometheus-k8s  2.33.5   active      2  prometheus-k8s  stable   103  10.152.183.82  no

Unit               Workload  Agent  Address      Ports  Message
prometheus-k8s/0*  active    idle   10.1.97.236
prometheus-k8s/1   active    idle   10.1.97.238

Relation provider                Requirer                         Interface         Type  Message
prometheus-k8s:prometheus-peers  prometheus-k8s:prometheus-peers  prometheus_peers  peer

Storage Unit      Storage ID  Type        Pool        Mountpoint                        Size    Status    Message
prometheus-k8s/0  database/0  filesystem  kubernetes  /var/lib/juju/storage/database/0  1.0GiB  attached  Successfully provisioned volume pvc-f1549b23-7f2d-44c2-8bbb-87834316a
715
prometheus-k8s/1  database/1  filesystem  kubernetes  /var/lib/juju/storage/database/0  1.0GiB  attached  Successfully provisioned volume pvc-d31094c3-3deb-46c0-bdf8-179fa04b1
371

juju exec --unit prometheus-k8s/1 -- close-port 3000
this unit is not the leader

juju exec --unit prometheus-k8s/0 -- opened-ports
3000/tcp

juju exec --unit prometheus-k8s/1 -- opened-ports
3000/tcp

juju:PRIMARY> db.openedPorts.find().pretty()
{
	"_id" : "d83ce064-96e9-4521-82f6-bae42cd69d53:a#prometheus-k8s",
	"model-uuid" : "d83ce064-96e9-4521-82f6-bae42cd69d53",
	"application-name" : "prometheus-k8s",
	"port-ranges" : {
		"" : [
			{
				"fromport" : 3000,
				"toport" : 3000,
				"protocol" : "tcp"
			}
		]
	},
	"txn-revno" : 2
}

juju upgrade-controller -c k1 --agent-version 3.1.1
best version:
    3.1.1
started upgrade to 3.1.1

juju upgrade-model -m t1
best version:
    3.1.1
started upgrade to 3.1.1

juju:PRIMARY> db.openedPorts.find().pretty()
{
	"_id" : "d83ce064-96e9-4521-82f6-bae42cd69d53:a#prometheus-k8s",
	"model-uuid" : "d83ce064-96e9-4521-82f6-bae42cd69d53",
	"application-name" : "prometheus-k8s",
	"port-ranges" : {

	},
	"txn-revno" : NumberLong(3),
	"unit-port-ranges" : {
		"prometheus-k8s/0" : {
			"" : [
				{
					"fromport" : 3000,
					"toport" : 3000,
					"protocol" : "tcp"
				}
			]
		},
		"prometheus-k8s/1" : {
			"" : [
				{
					"fromport" : 3000,
					"toport" : 3000,
					"protocol" : "tcp"
				}
			]
		}
	}
}

juju exec --unit prometheus-k8s/0 -- opened-ports
3000/tcp

juju exec --unit prometheus-k8s/1 -- opened-ports
3000/tcp

juju exec --unit prometheus-k8s/1 -- close-port 3000

juju:PRIMARY> db.openedPorts.find().pretty()
{
	"_id" : "3899fbb4-bc47-43d6-82af-bf35dc626ac2:a#prometheus-k8s",
	"model-uuid" : "3899fbb4-bc47-43d6-82af-bf35dc626ac2",
	"application-name" : "prometheus-k8s",
	"port-ranges" : {

	},
	"txn-revno" : NumberLong(4),
	"unit-port-ranges" : {
		"prometheus-k8s/1" : {

		},
		"prometheus-k8s/0" : {
			"" : [
				{
					"fromport" : 3000,
					"toport" : 3000,
					"protocol" : "tcp"
				}
			]
		}
	}
}

juju exec --unit prometheus-k8s/0 -- opened-ports
3000/tcp

juju exec --unit prometheus-k8s/1 -- opened-ports

Documentation changes

Now, sidecar and machine applications are same for open[/close]-port

Bug reference

https://bugs.launchpad.net/juju/+bug/2007334

go.mod

state/migration_import.go

wallyworld

I think we can discuss naming things in the state layer. The methods and entities used can be confusing - eg we are modelling ports opened by a unit but referring to application ports. We have an application ports doc but we can have methods on it that real just with units right? I'm also confused about the additions to the all watcher.

api/agent/uniter/uniter.go

apiserver/facades/agent/uniter/uniter.go

apiserver/facades/agent/uniter/uniter_test.go

state/allwatcher.go

worker/uniter/runner/context/ports_test.go

apiserver/facades/controller/caasfirewaller/firewaller_test.go

state/machine_ports.go

state/unit.go

wallyworld

Thank you for all great changes. One issue though - I think we are missing the uniter facade bump since we are replacing a method.

api/agent/uniter/uniter.go

wallyworld · 2023-03-02T02:01:47Z

apiserver/facades/agent/uniter/uniter.go

+}
+
+// OpenedPortRangesByEndpoint returns the port ranges opened by the unit.
+func (u *UniterAPI) OpenedPortRangesByEndpoint() (params.OpenPortRangesByEndpointResults, error) {


Don't we need to bump the facade version?

This method is new and we keep the existing method OpenedApplicationPortRangesByEndpoint.
So it should be ok without a facade version bump?

wallyworld · 2023-03-02T02:05:38Z

apiserver/facades/agent/uniter/uniter.go

+		Results: make([]params.OpenPortRangesByEndpointResult, 1),
+	}
+
+	authTag := u.auth.GetAuthTag()


Aren't we missing a permission check like accessUnit() here?

This facade method does not have an arg, we are using the authTag to access its own unit.
So I think we are ok here.

wallyworld · 2023-03-02T02:14:28Z

testcharms/charm-repo/focal/cockroach/metadata.yaml

+peers:
+  ring:
+    interface: cockroachdb


Is this needed?

Yes, we do. This was added for the new sidecar test.#15225 (comment)

state/unit_ports.go

worker/uniter/runner/context/contextfactory.go

wallyworld · 2023-03-02T02:21:57Z

With the changes, would be worth testing again:

older model on newer controller
migration

ycliuhw · 2023-03-02T04:46:50Z

/build

…ned ports;

…gesForUnit to unitPortRanges;

…ranges;

ycliuhw · 2023-03-03T05:21:52Z

With the changes, would be worth testing again:

older model on newer controller

migration

Works fine to upgrade from 3.1.0 to 3.1.1.
But I can't fully test model migration because these two bugs:

wallyworld

I think we should move the app ports to each unit, not copy.
Every unit will typically open the same port in say a start hook. This will be how it works till we add --app later.

wallyworld · 2023-03-03T05:51:39Z

state/upgrades.go

+				Id: doc.DocID,
+				Update: bson.D{
+					{Name: "$set", Value: bson.D{
+						{Name: "port-ranges", Value: doc.PortRanges},


Do we need "port-ranges"? We are just treating the ports as being the same for all units right?
So we copy the ports to all units and leave any application ports empty till next cycle.

doc.PortRanges has already set to nil.

upgrades/steps_311.go

ycliuhw · 2023-03-03T09:40:08Z

/merge

#15263 Merge 3.1 #15220 [from manadart/2.9-revert-omit-match](4c1503b) #15201 [from toabctl/develop-aws-new-regions](125496e) #15196 [from hmlanigan/fix-refresh-resource-no-char…](344de3f) #15224 [from wallyworld/handle-terminated-hooks](c1618b5) #15238 [from hmlanigan/fix-local-refresh-with-resou…](7b871e7) #15247 [from hmlanigan/new-merge-from-2.9](7587d44) #15246 [from nvinuesa/juju-2951](3272e11) #15248 [from manadart/2.9-into-3.0](91165d3) #15241 [from hmlanigan/update-refresh-help](f0e4b1b) #15252 [from hmlanigan/merge-from-2.9](992f040) #15253 [from hpidcock/fix-install-step](95c2db7) #15254 [from wallyworld/merge-3.0-20230303](ce6a255) #15225 [from ycliuhw/open-port-per-unit4sidecar](7dfbd61) #15256 [from jack-w-shaw/JUJU-2899_fix_bug_in_ec2_t…](1b2c08f) #15257 [from jack-w-shaw/2.9-into-3.0](bfaa9ed) #15228 [from jack-w-shaw/JUJU-2897_back_ssh_allowli…](18349be) #15261 [from wallyworld/secret-consumer-watcher-fix](1837f71) #15262 [from wallyworld/merge-3.0-20230307](4451c9c) Conflicts were for deleted code or code that needed to be removed. ``` # Conflicts: # api/client/firewallrules/client_test.go # api/client/firewallrules/package_test.go # state/upgrades.go # state/upgrades_test.go # upgrades/backend.go # upgrades/steps_311.go # upgrades/steps_311_test.go ```

ycliuhw commented Feb 24, 2023

View reviewed changes

go.mod Outdated Show resolved Hide resolved

ycliuhw changed the title ~~Open port per unit4sidecar~~ [JUJU-2847] Open port per unit4sidecar Feb 24, 2023

ycliuhw force-pushed the open-port-per-unit4sidecar branch 3 times, most recently from ab387dd to 14c9dbd Compare February 27, 2023 05:19

manadart reviewed Feb 27, 2023

View reviewed changes

state/migration_import.go Outdated Show resolved Hide resolved

ycliuhw force-pushed the open-port-per-unit4sidecar branch 2 times, most recently from 01ba750 to 86dd3f8 Compare February 28, 2023 06:14

wallyworld reviewed Feb 28, 2023

View reviewed changes

ycliuhw force-pushed the open-port-per-unit4sidecar branch 3 times, most recently from 735adc3 to 1df2780 Compare March 1, 2023 07:01

ycliuhw requested a review from wallyworld March 1, 2023 22:27

wallyworld approved these changes Mar 2, 2023

View reviewed changes

ycliuhw force-pushed the open-port-per-unit4sidecar branch 2 times, most recently from d5d2924 to 6c0aa8f Compare March 2, 2023 03:28

ycliuhw force-pushed the open-port-per-unit4sidecar branch 2 times, most recently from be1d5bd to 9b813e9 Compare March 3, 2023 04:31

ycliuhw added 10 commits March 3, 2023 15:35

Change opened ports for application to be grouped by unit;

887fac4

Make allwatcher to handle application port changes;

3381cff

Add application port support for model migration;

3e6e69e

Change application port related facade to use unitTag;

c7aebe7

Change application port related hook context utils to use unitTag;

a14c6b5

Bump github.com/juju/description/v4 to v4.0.6;

bddd2b8

Remove debug log;

514364d

Add new facade method OpenedPortRangesByEndpoint for uniters;

7f08b33

Change to use OpenedPortRangesByEndpoint for fetching application ope…

079109b

…ned ports;

Rename unitPortRanges to unitPortRangesForMachine, applicationPortRan…

ed5d1bb

…gesForUnit to unitPortRanges;

ycliuhw added 2 commits March 3, 2023 15:37

Add peer relation to the cockroach testing charm;

b278e27

Add TestReadRemoteSettingsForCAASApplicationInPeerRelationSidecar;

2daf739

ycliuhw force-pushed the open-port-per-unit4sidecar branch from 9b813e9 to 06e8c21 Compare March 3, 2023 04:48

Add upgrade steps for migrating application port ranges to unit port …

10de1fe

…ranges;

ycliuhw force-pushed the open-port-per-unit4sidecar branch from 06e8c21 to 10de1fe Compare March 3, 2023 05:16

wallyworld approved these changes Mar 3, 2023

View reviewed changes

ycliuhw force-pushed the open-port-per-unit4sidecar branch from ff24f25 to 37e9de9 Compare March 3, 2023 05:57

Fix TestOpenedPortRanges;

d79bab1

ycliuhw force-pushed the open-port-per-unit4sidecar branch from 37e9de9 to d79bab1 Compare March 3, 2023 08:03

jujubot merged commit 7dfbd61 into juju:3.1 Mar 3, 2023

wallyworld mentioned this pull request Mar 7, 2023

Merge 3.1 #15263

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[JUJU-2847] Open port per unit4sidecar #15225

[JUJU-2847] Open port per unit4sidecar #15225

ycliuhw commented Feb 24, 2023 •

edited

wallyworld left a comment

wallyworld left a comment

wallyworld Mar 2, 2023

ycliuhw Mar 2, 2023

wallyworld Mar 2, 2023

ycliuhw Mar 2, 2023

wallyworld Mar 2, 2023

ycliuhw Mar 2, 2023

wallyworld commented Mar 2, 2023

ycliuhw commented Mar 2, 2023

ycliuhw commented Mar 3, 2023

wallyworld left a comment

wallyworld Mar 3, 2023

ycliuhw Mar 3, 2023

ycliuhw commented Mar 3, 2023

[JUJU-2847] Open port per unit4sidecar #15225

[JUJU-2847] Open port per unit4sidecar #15225

Conversation

ycliuhw commented Feb 24, 2023 • edited

Checklist

QA steps

Documentation changes

Bug reference

wallyworld left a comment

Choose a reason for hiding this comment

wallyworld left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wallyworld commented Mar 2, 2023

ycliuhw commented Mar 2, 2023

ycliuhw commented Mar 3, 2023

wallyworld left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ycliuhw commented Mar 3, 2023

ycliuhw commented Feb 24, 2023 •

edited