HTTP Proxy customer cases update #14513

vsedmik · 2024-03-25T15:04:01Z

This PR just addresses a few changes from the component eval

adds a case for insights client registration via HTTP proxy
removes old stub covered in foreman-maintain
updates http_proxy fixture to remove the proxy on teardown

PRT test Cases example

trigger: test-robottelo
pytest: tests/foreman/cli/test_http_proxy.py -k insights

vsedmik · 2024-03-25T15:04:12Z

trigger: test-robottelo
pytest: tests/foreman/cli/test_http_proxy.py -k insights

Satellite-QE · 2024-03-25T16:06:36Z

PRT Result

Build Number: 6186
Build Status: SUCCESS
PRT Comment: pytest tests/foreman/cli/test_http_proxy.py -k insights --external-logging
Test Result : ========== 6 passed, 3 deselected, 428 warnings in 3326.86s (0:55:26) ==========

tests/foreman/cli/test_http_proxy.py

Gauravtalreja1 · 2024-03-26T12:40:44Z

tests/foreman/cli/test_http_proxy.py

-@pytest.mark.tier3
-def test_positive_environment_variable_unset_set():
-    """Verify that satellite installer unsets and then sets back the environment variables
-
-    :id: 596d753b-660b-49cb-b663-ff3cec439564
-
-    :BZ: 1886040
-
-    :customerscenario: true
-


I remember this test, its related to old feature added in installer, which identifies and fails, if proxy is set on system, with this mentioned environment variable. So, I understand that this test is valid, but can I ask why are you considering removing it instead of automating it?

I considered removing because the BZ says

3. Why does the customer need this? (List the business requirements here) Since satellite maintain takes care of end to end upgrade, it should also have an additional check to unset an environment variable along with alerting the user. The addition of this check will ensure that we can avoid the installer execution failure and fixing a potential issue at the pre-upgrade stage.

This coverage already exists here, so I considered it redundant.

I don't see it as redundant as the test you linked is being verified by foreman-maintain check which verifies it it exists in the environment, and here we use the installer to test, where behaviour here is different, installers unsets the variable while running and then sets it back when installer run completes

Well, my assumptions were following:

The foreman-maintain health check (covered by the test linked above) checks if HTTP_PROXY env var is set and fails if it is.

Satellite upgrade leverages the foremain-maintain health check as pre-upgrade check.

If the pre-upgrade check fails (e.g. because of the HTTP_PROXY env var is set or otherwise), no upgrade nor satellite-installer is run. So the satellite-installer unset/set functionality is unreachable and thus appears redundant to me.

Is any of the assumptions wrong?

Gauravtalreja1 · 2024-03-26T12:42:48Z

tests/foreman/cli/test_http_proxy.py

+        org=rhcloud_manifest_org.label,
+        rhel_distro=f"rhel{rhel_contenthost.os_version.major}",
+    )
+    assert rhel_contenthost.execute('insights-client --register').status == 0


configure_rhai_client() will register this client to satellite, so here are we checking if it could be registered again to same satelllite? if not, can we remove this assertion?

This is correct, I think testing re-register won't harm.

Gauravtalreja1 · 2024-03-26T12:44:55Z

tests/foreman/cli/test_http_proxy.py

-        1. Create HTTP Proxy.
-        2. Set created proxy as "Default HTTP Proxy" in settings.
-        3. Edit /etc/resolv.conf and comment out all entries so that
-            satellite can not directly communicate outside. Ensure that
-            NetworkManger won't change it.


As we're still following steps as part of setup with fixture, so can we keep this steps?

Well, yes and no. Right now we don't do this kind of setup for any HTTP proxy testing, so this would be an exception. If we decide to hack the Satellite like this, I would rather do it for all tests through some fixture, not just for this particular case. It will require more testing for potential side-effects, though.

@Gauravtalreja1 PoC for the above #14754

pondrejk

ack pending other reviewer's comments

damoore044

Nice! ack pending prior comments

This commit: - adds a case for insights client registration via HTTP proxy - removes old stub covered in foreman-maintain - updates http_proxy fixture to remove the proxy on teardown

vsedmik · 2024-05-02T14:19:49Z

rebased

vsedmik · 2024-05-02T14:21:50Z

trigger: test-robottelo
pytest: tests/foreman/cli/test_http_proxy.py -k insights

Satellite-QE · 2024-05-02T15:15:28Z

PRT Result

Build Number: 6779
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/cli/test_http_proxy.py -k insights --external-logging
Test Result : ===== 3 failed, 3 passed, 3 deselected, 479 warnings in 2910.62s (0:48:30) =====

Satellite-QE · 2024-05-06T08:01:53Z

PRT Result

Build Number: 6793
Build Status: SUCCESS
PRT Comment: pytest tests/foreman/cli/test_http_proxy.py -k insights --external-logging
Test Result : ========== 6 passed, 3 deselected, 495 warnings in 2965.79s (0:49:25) ==========

This commit: - adds a case for insights client registration via HTTP proxy - removes old stub covered in foreman-maintain - updates http_proxy fixture to remove the proxy on teardown (cherry picked from commit c4507eb)

This commit: - adds a case for insights client registration via HTTP proxy - removes old stub covered in foreman-maintain - updates http_proxy fixture to remove the proxy on teardown

vsedmik added CherryPick PR needs CherryPick to previous branches 6.14.z Introduced in or relating directly to Satellite 6.14 6.15.z Introduced in or relating directly to Satellite 6.15 labels Mar 25, 2024

vsedmik self-assigned this Mar 25, 2024

vsedmik requested review from a team as code owners March 25, 2024 15:04

Satellite-QE added the PRT-Passed Indicates that latest PRT run is passed for the PR label Mar 25, 2024

Gauravtalreja1 reviewed Mar 26, 2024

View reviewed changes

pondrejk approved these changes Mar 27, 2024

View reviewed changes

damoore044 approved these changes Apr 2, 2024

View reviewed changes

sambible approved these changes Apr 3, 2024

View reviewed changes

Gauravtalreja1 added the AutoMerge_Cherry_Picked The cherrypicked PRs of master PR would be automerged if all checks passing label Apr 9, 2024

HTTP Proxy customer cases update

de79b85

This commit: - adds a case for insights client registration via HTTP proxy - removes old stub covered in foreman-maintain - updates http_proxy fixture to remove the proxy on teardown

vsedmik force-pushed the http-insights-client branch from aa2a952 to de79b85 Compare May 2, 2024 14:19

Satellite-QE removed the PRT-Passed Indicates that latest PRT run is passed for the PR label May 2, 2024

Satellite-QE added the PRT-Failed Indicates that latest PRT run is failed for the PR label May 2, 2024

Satellite-QE added PRT-Passed Indicates that latest PRT run is passed for the PR and removed PRT-Failed Indicates that latest PRT run is failed for the PR labels May 6, 2024

pondrejk merged commit c4507eb into SatelliteQE:master May 16, 2024
13 checks passed

This was referenced May 16, 2024

[6.14.z] HTTP Proxy customer cases update #15081

Merged

[6.15.z] HTTP Proxy customer cases update #15082

Merged

vsedmik mentioned this pull request May 30, 2024

Fix HTTP proxy teardown #15229

Merged

This was referenced May 30, 2024

[6.14.z] Fix HTTP proxy teardown #15234

Merged

[6.15.z] Fix HTTP proxy teardown #15235

Merged

[6.16.z] Fix HTTP proxy teardown #15236

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HTTP Proxy customer cases update #14513

HTTP Proxy customer cases update #14513

vsedmik commented Mar 25, 2024

vsedmik commented Mar 25, 2024

Satellite-QE commented Mar 25, 2024

Gauravtalreja1 Mar 26, 2024

vsedmik Mar 27, 2024

Gauravtalreja1 Apr 9, 2024

vsedmik Apr 9, 2024

Gauravtalreja1 Mar 26, 2024

vsedmik Mar 27, 2024

Gauravtalreja1 Mar 26, 2024

vsedmik Mar 27, 2024

vsedmik Apr 12, 2024

pondrejk left a comment

damoore044 left a comment

vsedmik commented May 2, 2024

vsedmik commented May 2, 2024

Satellite-QE commented May 2, 2024

Satellite-QE commented May 6, 2024

HTTP Proxy customer cases update #14513

HTTP Proxy customer cases update #14513

Conversation

vsedmik commented Mar 25, 2024

This PR just addresses a few changes from the component eval

PRT test Cases example

vsedmik commented Mar 25, 2024

Satellite-QE commented Mar 25, 2024

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pondrejk left a comment

Choose a reason for hiding this comment

damoore044 left a comment

Choose a reason for hiding this comment

vsedmik commented May 2, 2024

vsedmik commented May 2, 2024

Satellite-QE commented May 2, 2024

Satellite-QE commented May 6, 2024