Restart QDR after changing the password

[csibbitt]

• Fixes bug reported here: QDR Auth for infrared 17.1 script #517 (comment)
• Avoids an extra manual step when password changes
• Would affect users who upgrade from earlier STF and subsequently enable basic auth
• Also users who need to change their passwords

[csibbitt]

Testing

Before

I have a working system, then I regenerate the password by removing the stf_one_time_upgrade label and deleting the STO pod (to trigger an operator run), after that the system is not working.

$ ../tests/smoketest/smoketest.sh 
[...]
*** [SUCCESS] Smoke test job completed successfully

$ oc patch secret default-interconnect-users --type=json --patch '[{"op": "remove", "path":"/metadata/labels/stf_one_time_upgrade"}]' 
secret/default-interconnect-users patched

$ oc delete po -l name=service-telemetry-operator
pod "service-telemetry-operator-54cd5c8dd-rh8rs" deleted

$ oc get secret default-interconnect-users -o yaml | grep stf_one_time
    stf_one_time_upgrade: "1699468854"

$ ../tests/smoketest/smoketest.sh 
[...]
*** [FAILURE] Smoke test job still not succeeded after 300s

Deleting the QDR pod makes it work again

$ oc delete po -l application=default-interconnect
pod "default-interconnect-7c77794bf7-tjqj9" deleted

$ ./tests/smoketest/smoketest.sh 
[...]
*** [SUCCESS] Smoke test job completed successfully

After

Get working system

$ git reflog | head -1
5ada4e9 HEAD@{0}: commit (amend): Restart QDR after changing the password

$ oc delete project service-telemetry
project.project.openshift.io "service-telemetry" deleted

$ oc new-project service-telemetry
[...]

$ ansible-playbook --extra-vars __local_build_enabled=true --extra-vars working_branch="$(git rev-parse --abbrev-ref HEAD)" --extra-vars __service_telemetry_snmptraps_enabled=true --extra-vars __service_telemetry_storage_ephemeral_enabled=true --extra-vars __service_telemetry_observability_strategy=use_redhat ./run-ci.yaml
[...]

$ ../tests/smoketest/smoketest.sh 
[...]
*** [SUCCESS] Smoke test job completed successfully

Stays working after password regeneration, with no QDR pod restart required

$ oc patch secret default-interconnect-users --type=json --patch '[{"op": "remove", "path":"/metadata/labels/stf_one_time_upgrade"}]' 
secret/default-interconnect-users patched

$ oc delete po -l name=service-telemetry-operator
pod "service-telemetry-operator-6d8b6797b7-k2sqf" deleted

$ oc get secret default-interconnect-users -o yaml | grep stf_one_time
    stf_one_time_upgrade: "1699547215"

$ ../tests/smoketest/smoketest.sh 
[...]
*** [SUCCESS] Smoke test job completed successfully

[leifmadsen]

Approving because this looks good, but a couple minor comments.

[vkmc]

Code looks good, I wouldn't worry too much about HA is we are planning to deprecate soon

[csibbitt]

HA Testing

Rebuild with latest changes

$ git reflog | head -1
5cd57e1 HEAD@{0}: commit (amend): Adjust QDR restarts to account for HA

$ oc start-build service-telemetry-operator --wait --from-dir . ; oc delete pod --selector=name=service-telemetry-operator
[...]

Test password regeneration (and restart) has no errors when no Interconnect pods exist

$ oc get po -l application=default-interconnect
NAME                                    READY   STATUS    RESTARTS   AGE
default-interconnect-7c77794bf7-x4m7s   1/1     Running   0          4m

$ oc delete interconnect default-interconnect; oc delete po -l application=default-interconnect
interconnect.interconnectedcloud.github.io "default-interconnect" deleted
pod "default-interconnect-7c77794bf7-x4m7s" deleted

# STO converges without intervention because it watches the Interconnect object
# On first pass it creates the Interconnect object but there was no users secret to upgrade yet
# The pod starts up

$ oc get po -l application=default-interconnect
NAME                                    READY   STATUS    RESTARTS   AGE
default-interconnect-7c77794bf7-8x8q2   1/1     Running   0          12s

# On second pass it upgrades the secret and restarts the pod

$ oc get secret default-interconnect-users -o yaml | grep stf_one_time
    stf_one_time_upgrade: "1699561745"

$ oc get po -l application=default-interconnect
NAME                                    READY   STATUS    RESTARTS   AGE
default-interconnect-7c77794bf7-8x8q2   1/1     Running   0          6s

Enable HA to get multiple pods and ensure they restart when we regenerate the password

$ oc patch stf default --patch '{"spec":{"highAvailability":{"enabled": true}}}' --type=merge
servicetelemetry.infra.watch/default patched

$ oc get po -l application=default-interconnect
NAME                                    READY   STATUS    RESTARTS   AGE
default-interconnect-7c77794bf7-8x8q2   1/1     Running   0          6m21s
default-interconnect-7c77794bf7-c8xjr   1/1     Running   0          2m12s

$ oc patch secret default-interconnect-users --type=json --patch '[{"op": "remove", "path":"/metadata/labels/stf_one_time_upgrade"}]'
secret/default-interconnect-users patched

$ oc delete po -l name=service-telemetry-operator
pod "service-telemetry-operator-6d8b6797b7-pjrq5" deleted

$ oc get po -l application=default-interconnect
NAME                                    READY   STATUS    RESTARTS   AGE
default-interconnect-7c77794bf7-2bm6d   1/1     Running   0          6s
default-interconnect-7c77794bf7-dpb7r   1/1     Running   0          5s

[ayefimov-1]

Seems good to me.

[csibbitt]

The 4.14 tests are failing from what looks like a race condition. I have a patch coming that will wait for the qdr-test to be Running, which I hope will fix it.

[csibbitt]

Okay, there is actually a SECOND race condition now. Sometimes the qdr-test configmap is created before the password gets upgraded, and so fails to connect to an upgraded/restarted STF QDR. This was actually masked before when the QDR was not being restarted since they both had the old password and would connect using it. I'll keep working on this branch to solve this so that we don't have additional backports.

[softwarefactory-project-zuul]

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://review.rdoproject.org/zuul/buildset/629ee486ebce48e3aeee8bb020b8e707

❌ stf-crc-latest-nightly_bundles TIMED_OUT in 1h 00m 48s
❌ stf-crc-latest-local_build TIMED_OUT in 1h 00m 50s

[csibbitt]

recheck

[csibbitt]

This passed the 4.12 and 4.14 Jenkins tests 3 times in a row (each), but the zuul one timed out waiting for the password to upgrade. Without an STO log I'm not really sure why that might happen.

[csibbitt]

The previous r3check caused the 4.12 jenkins job to run twice (once in branch mode, once in merge mode maybe?). So now the Jenkins' have run this 8 times in a row with no trouble, but zuul times out waiting on this line: d0d5460#diff-16335c82c44304ee49855ecebf854c2a8db384a1240f299c5f023b4b9845e407R64

That's the exact same expression as was previously used to capture the password, and it passed in previous iterations of this PR. Not sure what's going on here, but will probably need to extend the logging to get enough information to debug.

[softwarefactory-project-zuul]

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://review.rdoproject.org/zuul/buildset/6dc66d329b15441a9da68059ab49fa8c

❌ stf-crc-latest-nightly_bundles TIMED_OUT in 1h 00m 51s
❌ stf-crc-latest-local_build TIMED_OUT in 1h 00m 50s

[csibbitt]

Oh! It seems that zuul is still testing with auth: none[1] which apparently worked fine until we started waiting around for behavior that only executes when using auth: basic! I'm not sure where that's slipping in, as I thought it was just using defaults the whole way through (since we removed the override in stf-v-c MR 103).

CC: @elfiesmelfie any ideas?

[1] https://review.rdoproject.org/zuul/build/1f7d700e607947cf833640fc48b7bf3e/log/controller/smoketest.log#1718

EDIT: I found it! https://github.com/infrawatch/service-telemetry-operator/blob/master/ci/vars-zuul-common.yml#L5

[leifmadsen]

Oops! I fat fingered a button. Sorry!

[elfiesmelfie]

Oh! It seems that zuul is still testing with auth: none[1] which apparently worked fine until we started waiting around for behavior that only executes when using auth: basic! I'm not sure where that's slipping in, as I thought it was just using defaults the whole way through (since we removed the override in stf-v-c MR 103).

CC: @elfiesmelfie any ideas?

[1] https://review.rdoproject.org/zuul/build/1f7d700e607947cf833640fc48b7bf3e/log/controller/smoketest.log#1718

EDIT: I found it! https://github.com/infrawatch/service-telemetry-operator/blob/master/ci/vars-zuul-common.yml#L5

Yup, I added that when the default became "auth: basic" and the smoke tests stopped passing.