external: fix quote parsing and message in import-external-cluster.sh #16646
Conversation
|
Hello @parth-gr |
| if [ -n "$RADOS_NAMESPACE" ]; then | ||
| createRadosNamespaceCR | ||
| timeout 20 sh -c "until [ $($KUBECTL -n "$NAMESPACE" get CephBlockPoolRadosNamespace/"$RADOS_NAMESPACE" -o jsonpath='{.status.phase}' | grep -c "Ready") -eq 1 ]; do echo "waiting for radosNamespace to get created" && sleep 1; done" | ||
| timeout 20 sh -c "until [ \$($KUBECTL -n \"$NAMESPACE\" get CephBlockPoolRadosNamespace/\"$RADOS_NAMESPACE\" -o jsonpath='{.status.phase}' | grep -c 'Ready') -eq 1 ]; do echo 'waiting for radosNamespace to get created' && sleep 1; done" |
There was a problem hiding this comment.
the earlier version of script work well for me,
But at the same time the changes look good,
Can you share the sample output of the RADOS_NAMESPACE, that you are exporting
There was a problem hiding this comment.
Since I re-initialized and reset the Rook-Ceph cluster, I can no longer verify the exact RADOS_NAMESPACE value that was used previously.
However, I was able to confirm the SUBVOLUME_GROUP value, as it is still in use.
When generated via create-external-cluster-resources.py, the output was:
export SUBVOLUME_GROUP=external
Using this value and running import-external-cluster.sh resulted in the same error described in the issue:
Syntax error: end of file unexpected (expecting "done")
Additionally, the structure of the script that waits for resources to become ready—specifically the sections handling both:
• CephBlockPoolRadosNamespace/"$RADOS_NAMESPACE"
• CephFilesystemSubVolumeGroup/"$SUBVOLUME_GROUP"
—is identical inside the importClusterID function, and the root cause is the same quotation parsing issue.
Because of this, I believe the same fix should be applied to both parts.
Thanks for your time.
There was a problem hiding this comment.
The changes make sense to me, so the evaluation of the variables is deferred until their execution instead of being evaluated immediately when the script is parsed.
| if [ -n "$SUBVOLUME_GROUP" ]; then | ||
| createSubvolumeGroupCR | ||
| timeout 20 sh -c "until [ $($KUBECTL -n "$NAMESPACE" get CephFilesystemSubVolumeGroup/"$SUBVOLUME_GROUP" -o jsonpath='{.status.phase}' | grep -c "Ready") -eq 1 ]; do echo "waiting for radosNamespace to get created" && sleep 1; done" | ||
| timeout 20 sh -c "until [ \$($KUBECTL -n \"$NAMESPACE\" get CephFilesystemSubVolumeGroup/\"$SUBVOLUME_GROUP\" -o jsonpath='{.status.phase}' | grep -c 'Ready') -eq 1 ]; do echo 'waiting for subVolumeGroup to get created' && sleep 1; done" |
There was a problem hiding this comment.
Do you really need to add the quotes around \"$SUBVOLUME_GROUP\"? Seems like that shouldn't be necessary since the CR name will not have spaces.
There was a problem hiding this comment.
Yes, that makes sense. I think I can remove the quotes and use $SUBVOLUME_GROUP directly.
The same applies to the CephBlockPoolRadosNamespace/\"$RADOS_NAMESPACE\" part. If that looks good, I’ll update both accordingly. Thank you!
Fixed a syntax error caused by incorrect quotation parsing in the importClusterID function of import-external-cluster.sh, and updated the output message shown while waiting for the SubVolumeGroup CR to be created. Signed-off-by: GanghyeonSeo <sgh000109@gmail.com>
|
@travisn |
|
The CI issue is unrelated |
external: fix quote parsing and message in import-external-cluster.sh (backport #16646)
| if [ -n "$RADOS_NAMESPACE" ]; then | ||
| createRadosNamespaceCR | ||
| timeout 20 sh -c "until [ $($KUBECTL -n "$NAMESPACE" get CephBlockPoolRadosNamespace/"$RADOS_NAMESPACE" -o jsonpath='{.status.phase}' | grep -c "Ready") -eq 1 ]; do echo "waiting for radosNamespace to get created" && sleep 1; done" | ||
| timeout 20 sh -c "until [ \$($KUBECTL -n \"$NAMESPACE\" get CephBlockPoolRadosNamespace/$RADOS_NAMESPACE -o jsonpath='{.status.phase}' | grep -c 'Ready') -eq 1 ]; do echo 'waiting for radosNamespace to get created' && sleep 1; done" |
There was a problem hiding this comment.
@travisn @GanghyeonSeo mine question is why not to delay rados namespace expansion too?
\"\$RADOS_NAMESPACE\"
Another question, as the variables are defined in the outer shell so why we need to delay the expansion?
There was a problem hiding this comment.
It works without errors in its current form, after reconsidering based on your feedback, I came to the following conclusion:
Since the variables are defined in the outer shell and the value for KUBECTL is assigned inside the shell script, $NAMESPACE, $RADOS_NAMESPACE, and $SUBVOLUME_GROUP do not require delayed expansion — The script runs correctly without delayed expansion.
However, the kubectl invocation itself must remain escaped as \$($KUBECTL ...) to ensure it is evaluated in the inner shell.
Do you think strict consistency in quotation style is necessary here?
If so, for the following part:
-n \"$NAMESPACE\" get CephBlockPoolRadosNamespace/$RADOS_NAMESPACE
Which of the following would be better to standardize?
-n "$NAMESPACE" get CephBlockPoolRadosNamespace/"$RADOS_NAMESPACE"-n $NAMESPACE get CephBlockPoolRadosNamespace/$RADOS_NAMESPACE
This would also apply to the SubvolumeGroup part as well.
The existing escaping for \$($KUBECTL ...) and the quotation style(single quotation) in the waiting log message should remain unchanged. Thanks.
There was a problem hiding this comment.
First one looks good
-n "$NAMESPACE" get CephBlockPoolRadosNamespace/"$RADOS_NAMESPACE"
There was a problem hiding this comment.
Got it. I will proceed with the changes based on this and open a new PR.
Thank you for your guidance.
There was a problem hiding this comment.
I’ve applied the changes and opened a new PR.
You can find it here: #16706
Thanks!
Fixed an issue where running
import-external-cluster.shin the Rook consumer cluster would result in afor: 1: Syntax error: end of file unexpected (expecting "done")error when the exported config from the Ceph provider cluster (generated via create-external-cluster-resources.py) includes RADOS_NAMESPACE and SUBVOLUME_GROUP values.Additionally, updated the message shown while waiting for the CephFilesystemSubVolumeGroup CR to reach the Ready status.
Related to the issue below:
#16645
Checklist: