external: create a configmap for the command line args #14354

parth-gr · 2024-06-18T14:59:14Z

user can look back to there configurations by
looking at the configmap created with command
line arguments
This will be useful for them during upgrades when they need to re run the python script with the same flags

Checklist:

Commit Message Formatting: Commit titles and messages follow guidelines in the developer guide.
Reviewed the developer guide on Submitting a Pull Request
Pending release notes updated with breaking and/or notable changes for the next minor release.
Documentation has been updated, if necessary.
Unit tests have been added, if necessary.
Integration tests have been added, if necessary.

parth-gr · 2024-06-19T06:54:00Z

I have only added the arguments that has some value

[{"name": "external-cluster-user-command", "kind": "ConfigMap", "data": {"data": ["verbose: False", "ceph-conf: None", "keyring: None", "rgw-pool-prefix: default", "restricted-auth-permission: False", "v2-port-enable: False", "format: json", "cephfs-filesystem-name: myfs", "cephfs-metadata-pool-name: myfs-metadata", "cephfs-data-pool-name: myfs-replicated", "rbd-data-pool-name: replicapool", "rgw-skip-tls: False", "skip-monitoring-endpoint: False", "dry-run: False", "upgrade: False"]}}, {"name": "rook-ceph-mon-endpoints", "kind": "ConfigMap", "data": {"data": "a=10.97.22.211:6789", "maxMonId": "0", "mapping": "{}"}}, {"name": "rook-ceph-mon", "kind": "Secret", "data": {"admin-secret": "admin-secret", "fsid": "cacf16f1-a616-4fa7-abed-e0b373739a12", "mon-secret": "mon-secret"}}, {"name": "rook-ceph-operator-creds", "kind": "Secret", "data": {"userID": "client.healthchecker", "userKey": "AQAgf3Jmrz0SGBAAdRNMCORxP6tvQA80QX9jUQ=="}}, {"name": "monitoring-endpoint", "kind": "CephCluster", "data": {"MonitoringEndpoint": "10.244.239.85", "MonitoringPort": "9283"}}, {"name": "rook-csi-rbd-node", "kind": "Secret", "data": {"userID": "csi-rbd-node", "userKey": "AQBDcnJmycs5ExAAWK+bf6EQ5mjOhNUHo6cjOQ=="}}, {"name": "rook-csi-rbd-provisioner", "kind": "Secret", "data": {"userID": "csi-rbd-provisioner", "userKey": "AQBDcnJml+NaBBAAynhQU44b416JvlgRla5Bpw=="}}, {"name": "rook-csi-cephfs-provisioner", "kind": "Secret", "data": {"adminID": "csi-cephfs-provisioner", "adminKey": "AQBDcnJmp8xFJRAA07NNBroRhS9zsSwT/Nw7vA=="}}, {"name": "rook-csi-cephfs-node", "kind": "Secret", "data": {"adminID": "csi-cephfs-node", "adminKey": "AQBDcnJmKaAyNRAAX9u7L0DT2htQjKTwygOHhA=="}}, {"name": "rook-ceph-dashboard-link", "kind": "Secret", "data": {"userID": "ceph-dashboard-link", "userKey": "http://10.244.239.85:7000/"}}, {"name": "ceph-rbd", "kind": "StorageClass", "data": {"pool": "replicapool", "csi.storage.k8s.io/provisioner-secret-name": "rook-csi-rbd-provisioner", "csi.storage.k8s.io/controller-expand-secret-name": "rook-csi-rbd-provisioner", "csi.storage.k8s.io/node-stage-secret-name": "rook-csi-rbd-node"}}, {"name": "cephfs", "kind": "StorageClass", "data": {"fsName": "myfs", "pool": "myfs-replicated", "csi.storage.k8s.io/provisioner-secret-name": "rook-csi-cephfs-provisioner", "csi.storage.k8s.io/controller-expand-secret-name": "rook-csi-cephfs-provisioner", "csi.storage.k8s.io/node-stage-secret-name": "rook-csi-cephfs-node"}}]

parth-gr · 2024-06-19T15:22:01Z

@BlaineEXE For the command output in config map,

Which way we should output:

Lets say command is: python3 a.py --abc 123 --pqr 456

The exact command
"cmd": "python3 a.py --abc 123 --pqr 456"
Or individual flags,

"abc": 123
"pqr": 456

BlaineEXE · 2024-06-19T18:35:11Z

In order to facilitate user ease of use, I think it makes most sense to store things in a format that is immediately usable without users having to do anything. For storing configs based on CLI flags, I think it makes the most sense to store them in flag form like --abc 123 --pqr 456. I think that it probably makes the most sense to record only the flags themselves and not the command.

Assuming work is completed to implement config file support, we can instead output a generated config file, which might be more legible like your second example but still easily usable.

parth-gr · 2024-06-20T14:23:35Z

Output: {"name": "external-cluster-user-command", "kind": "ConfigMap", "data": {"data": "--rbd-data-pool-name replicapool"}}

BlaineEXE · 2024-06-20T15:05:53Z

Do we need any upstream doc updates for this?

parth-gr · 2024-06-21T12:53:27Z

Do we need any upstream doc updates for this?

@BlaineEXE that was a good reminder,

So I even created the configmap in the import script that upstream uses.

And then I realised we haven't document on, if we need to upgrade and utilise new feature we need to re-run the script, so I need to draw a attention first on how to utilise new flag on a brow field cluster, so I am creating a new issue in which I will document the same and also add about the command line args configmap

BlaineEXE · 2024-06-24T15:52:58Z

deploy/examples/import-external-cluster.sh

+      "$EXTERNAL_COMMAND_CONFIGMAP_NAME" \
+      --from-literal=command="$EXTERNAL_CLUSTER_USER_COMMAND" \
+      else
+    echo "configmap $EXTERNAL_COMMAND_CONFIGMAP_NAME already exists"


If it already exists, should we assume that this is a re-run with an update? In that case I think the tool should update the existing cm

That correct we need to patch it, thanks

deploy/examples/import-external-cluster.sh

deploy/examples/create-external-cluster-resources.py

BlaineEXE · 2024-06-24T18:11:07Z

deploy/examples/import-external-cluster.sh

+      "$EXTERNAL_COMMAND_CONFIGMAP_NAME" \
+      --from-literal=command="$EXTERNAL_CLUSTER_USER_COMMAND"
+  else
+    echo "configmap $EXTERNAL_COMMAND_CONFIGMAP_NAME already exists, update it"


Users may interpret "update it" to mean that they need to take action.

Suggested change

echo "configmap $EXTERNAL_COMMAND_CONFIGMAP_NAME already exists, update it"

echo "configmap $EXTERNAL_COMMAND_CONFIGMAP_NAME already exists, updating it"

BlaineEXE · 2024-06-24T18:12:29Z

deploy/examples/import-external-cluster.sh

+    $KUBECTL -n "$NAMESPACE" patch configmap "$EXTERNAL_COMMAND_CONFIGMAP_NAME" \
+      -p "{'data':{'command':$EXTERNAL_COMMAND_CONFIGMAP_NAME}}"


Let's try to be consistent with formatting. The $KUBECTL create X Y --vals commands elsewhere all use the format below. Let's do the same for this patch command to keep consistency.

$KUBECTL -n "$NAMESPACE" \ create \ X \ Y \ --vals

user can look back to there configurations by looking at the configmap created with command line arguments This will be useful for them during upgrades when they need to re run the python script with the same flags Signed-off-by: parth-gr <partharora1010@gmail.com>

travisn · 2024-07-01T20:51:31Z

@parth-gr What upstream documentation can we update so users will be aware of this new feature?

travisn · 2024-07-01T20:52:45Z

@parth-gr What upstream documentation can we update so users will be aware of this new feature?

Or maybe it's already covered by #14353?

BlaineEXE marked this pull request as draft June 18, 2024 15:52

parth-gr force-pushed the external-seamless-upgrade2 branch from adeffaa to d5cafa2 Compare June 19, 2024 06:54

parth-gr requested review from sp98, subhamkrai and BlaineEXE June 19, 2024 06:54

parth-gr marked this pull request as ready for review June 19, 2024 06:55

parth-gr force-pushed the external-seamless-upgrade2 branch from d5cafa2 to ec53413 Compare June 19, 2024 07:55

parth-gr force-pushed the external-seamless-upgrade2 branch 3 times, most recently from 8b59879 to 9560cdf Compare June 20, 2024 13:27

parth-gr force-pushed the external-seamless-upgrade2 branch from 9560cdf to feaf661 Compare June 21, 2024 12:51

This was referenced Jun 21, 2024

Document how to use new features with external mode #14368

Open

external: add support for config file in external mode #14353

Merged

BlaineEXE reviewed Jun 24, 2024

View reviewed changes

parth-gr force-pushed the external-seamless-upgrade2 branch from feaf661 to 6ab769b Compare June 24, 2024 16:23

parth-gr requested a review from BlaineEXE June 24, 2024 16:45

BlaineEXE requested changes Jun 24, 2024

View reviewed changes

parth-gr force-pushed the external-seamless-upgrade2 branch from 6ab769b to 951c683 Compare June 25, 2024 11:21

parth-gr requested a review from BlaineEXE June 25, 2024 11:21

BlaineEXE approved these changes Jun 25, 2024

View reviewed changes

BlaineEXE merged commit c85ce7c into rook:master Jun 25, 2024
52 of 53 checks passed

parth-gr added the backport-release-1.14 label Jun 25, 2024

mergify bot mentioned this pull request Jun 25, 2024

external: create a configmap for the command line args (backport #14354) #14380

Closed

6 tasks

parth-gr mentioned this pull request Jun 25, 2024

External: User should have the ability to update their config #14381

Open

BlaineEXE removed the backport-release-1.14 label Jun 26, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

external: create a configmap for the command line args #14354

external: create a configmap for the command line args #14354

parth-gr commented Jun 18, 2024

parth-gr commented Jun 19, 2024 •

edited

Loading

parth-gr commented Jun 19, 2024

BlaineEXE commented Jun 19, 2024

parth-gr commented Jun 20, 2024

BlaineEXE commented Jun 20, 2024

parth-gr commented Jun 21, 2024

BlaineEXE Jun 24, 2024

parth-gr Jun 24, 2024 •

edited

Loading

BlaineEXE Jun 24, 2024

BlaineEXE Jun 24, 2024

parth-gr Jun 25, 2024

travisn commented Jul 1, 2024

travisn commented Jul 1, 2024

	echo "configmap $EXTERNAL_COMMAND_CONFIGMAP_NAME already exists, update it"
	echo "configmap $EXTERNAL_COMMAND_CONFIGMAP_NAME already exists, updating it"

		$KUBECTL -n "$NAMESPACE" patch configmap "$EXTERNAL_COMMAND_CONFIGMAP_NAME" \
		-p "{'data':{'command':$EXTERNAL_COMMAND_CONFIGMAP_NAME}}"

external: create a configmap for the command line args #14354

external: create a configmap for the command line args #14354

Conversation

parth-gr commented Jun 18, 2024

parth-gr commented Jun 19, 2024 • edited Loading

parth-gr commented Jun 19, 2024

BlaineEXE commented Jun 19, 2024

parth-gr commented Jun 20, 2024

BlaineEXE commented Jun 20, 2024

parth-gr commented Jun 21, 2024

BlaineEXE Jun 24, 2024

Choose a reason for hiding this comment

parth-gr Jun 24, 2024 • edited Loading

Choose a reason for hiding this comment

BlaineEXE Jun 24, 2024

Choose a reason for hiding this comment

BlaineEXE Jun 24, 2024

Choose a reason for hiding this comment

parth-gr Jun 25, 2024

Choose a reason for hiding this comment

travisn commented Jul 1, 2024

travisn commented Jul 1, 2024

parth-gr commented Jun 19, 2024 •

edited

Loading

parth-gr Jun 24, 2024 •

edited

Loading