CASMPET-7117: iSCSI SBPS provision and DNS config fails when HSN is not configured/ CASMPET-7126: CFS play for k8s labelling fails when it is already applied

[ravikanth-nalla-hpe]

Summary and Scope

• CASMPET-7117: iSCSI SBPS: LIO provision and DNS records config fails when HSN is not configured

  • fixed iSCSI LIO provisioning to exclude HSN portal config when HSN n/w is not configured
  • fixed to avoid DNS "SRV" and "A" records creation for HSN when HSN is not configured

• CASMPET-7126: iSCSI SBPS: k8s labelling fails when it is already applied

  • fixed to avoid applying k8s label when it already exists

Issues and Related PRs

Original EPIC: https://jira-pro.it.hpe.com:8443/browse/CASMPET-6797

Testing

Tested on bare metal systems with multi worker nodes

Tested on:

"starlord" and "Odin"

Test description:

• verified LIO provisioning where HSN n/w is not configured, where it does not fail the overall config instead ignores HSN and creates rest of the portals (NMN and CMN).

• verified LIO provisioning where HSN n/w is configured, it creates all three n/w portals HSN, NMN and CMN.

• verified DNS "SRV" and "A" records creation where HSN n/w is not configured, it creates “SRV” and “A” records for NMN without any failure.

• verified DNS "SRV" and "A" records creation where HSN n/w is configured, it creates “SRV” and “A” records for both HSN and NMN without any failure.

• verified k8s node labelling fresh and also when it is already applied. K8s node labelling is successful without any failure in both cases.

• Were the install/upgrade-based validation checks/tests run (goss tests/install-validation doc)?

• Were continuous integration tests run? If not, why?

• Was upgrade tested? If not, why?

• Was downgrade tested? If not, why?

• Were new tests (or test issues/Jiras) created for this change?

Risks and Mitigations

None

Pull Request Checklist

• [] Version number(s) incremented, if applicable
• Copyrights updated
• [] License file intact
• Target branch correct
• CHANGELOG.md updated
• Testing is appropriate and complete, if applicable
• HPC Product Announcement prepared, if applicable

[dborman-hpe]

Previously I didn't notice all the hard-coded references to hpc.amslabs.hpecorp.net. Those all need to be fixed to dynamically determine the host's domain name and use that instead.

[dborman-hpe]

Please run the shell scripts through shellcheck and address the issues it finds (yes, I know some of the issues were there before, but it'd be good to clean them up)

[ravikanth-nalla-hpe]

Please run the shell scripts through shellcheck and address the issues it finds (yes, I know some of the issues were there before, but it'd be good to clean them up)

I have addressed all the "shellcheck" thrown warnings except the one below from file apply_k8s_label.sh

Line 29:
if [[ ! $(kubectl get nodes -l $LABEL | grep "$HOST_NAME") ]]
        ^-- SC2143 (style): Use grep -q instead of comparing output with [ -n .. ].
$

It is suggesting to use "-q" (quite option) but that does not work since the cmd returns "0" always (for both the cases where label is present / not present. Just that it won't print the output to stdout). Also, not opting to use "--overwrite" option just for the sake of avoiding this warning from "shellcheck". Since ansible is idempotent, just don't want to overwrite config.

[dborman-hpe]

Please run the shell scripts through shellcheck and address the issues it finds (yes, I know some of the issues were there before, but it'd be good to clean them up)

I have addressed all the "shellcheck" thrown warnings except the one below from file apply_k8s_label.sh

Line 29:
if [[ ! $(kubectl get nodes -l $LABEL | grep "$HOST_NAME") ]]
        ^-- SC2143 (style): Use grep -q instead of comparing output with [ -n .. ].
$

It is suggesting to use "-q" (quite option) but that does not work since the cmd returns "0" always (for both the cases where label is present / not present. Just that it won't print the output to stdout). Also, not opting to use "--overwrite" option just for the sake of avoiding this warning from "shellcheck". Since ansible is idempotent, just don't want to overwrite config.

It works fine for me:

ncn-m001:~ # kubectl get nodes -l $LABEL
NAME       STATUS   ROLES                  AGE   VERSION
ncn-m001   Ready    control-plane,master   13d   v1.22.13
ncn-w001   Ready    <none>                 13d   v1.22.13
ncn-w002   Ready    <none>                 13d   v1.22.13
ncn-w003   Ready    <none>                 13d   v1.22.13
ncn-w004   Ready    <none>                 13d   v1.22.13
ncn-m001:~ # kubectl get nodes -l $LABEL | grep -q ncn-w001
ncn-m001:~ # echo $?
0
ncn-m001:~ # kubectl get nodes -l $LABEL | grep -q ncn-m002
ncn-m001:~ # echo $?
1
ncn-m001:~ # 

You have to remove both $(...) and [[...]], so the change should be to simplify it to just:

if ! kubectl get nodes -l $LABEL | grep -q "$HOST_NAME"

[dborman-hpe]

@ravikanth-nalla-hpe, this looks good. Fix the final shellcheck error and this is ready to be merged, please squash everything down to a single commit.

[dborman-hpe]

LGTM, thanks!

[ravikanth-nalla-hpe]

@mharding-hpe Can you please merge this PR to master branch?

[mharding-hpe]

We don't want to check in commented lines. If this no longer is needed, we should just remove the line.

Suggested change

#- role: csm.sbps.mount_s3_images

[mharding-hpe]

Suggested change

	if ! kubectl get nodes -l $LABEL | grep -q "$HOST_NAME"
	then
	kubectl label nodes "$HOST_NAME" $LABEL
	fi
	# If the label is already applied to this node, then exit
	kubectl get nodes -l "$LABEL" --no-headers | grep -Eq "^${HOST_NAME}[[:space:]]" && exit 0
	# Otherwise, apply the label to the node
	kubectl label nodes "$HOST_NAME" "$LABEL"

[mharding-hpe]

@mharding-hpe Can you please merge this PR to master branch?

First we will need to merge it into develop, once we resolve the couple of small review comments. Then we can merge it to master.

[mharding-hpe]

You'll also need to rebase on top of the changes that have been made to develop