Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ENT-12151: Added possibility to configure Mission Portal web server ports #2947

Merged
merged 2 commits into from
Nov 4, 2024

Conversation

aleksandrychev
Copy link
Contributor

@aleksandrychev aleksandrychev commented Aug 29, 2024

Ticket: ENT-12151

Copy link
Member

@nickanderson nickanderson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I approve, but there is a suggestion of order change I think you should accept.

cfe_internal/enterprise/CFE_hub_specific.cf Outdated Show resolved Hide resolved
cfe_internal/enterprise/CFE_hub_specific.cf Show resolved Hide resolved
Copy link
Contributor

@craigcomstock craigcomstock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, distributed cleanup is the only thing that would have trouble if a feeder uses a different port and there isn't any way to know if THAT feeder over there, from the superhub perspective, has changed it's port or not.

I suppose we would have to include the port number in the information we keep on the superhub about the feeder.

As for the superhub accessing itself, that would need to be adjusted as well.

https://github.com/cfengine/masterfiles/blob/master/templates/federated_reporting/nova_api.py#L99

    def _request(self, method, path, body=None):
        url = "https://{}/api/{}".format(self._hostname, path)

@aleksandrychev
Copy link
Contributor Author

@craigcomstock I adjusted distributed_cleanup.py please review

Copy link
Contributor

@craigcomstock craigcomstock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say this second commit can be removed.

Copy link
Contributor

@craigcomstock craigcomstock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems pretty solid. I did the httpd.conf changes manually in the template myself in the case of running a VM so I can have multiple VM Hubs, aka port 8008 for rhel-8, 8016 for ubuntu-16, etc. Worked fine so I know the template portion is correct.

@aleksandrychev
Copy link
Contributor Author

aleksandrychev commented Sep 9, 2024

Build Status

@aleksandrychev
Copy link
Contributor Author

@cf-bottom jenkins

@cf-bottom
Copy link

@aleksandrychev
Copy link
Contributor Author

@cf-bottom jenkins

@cf-bottom
Copy link

@craigcomstock
Copy link
Contributor

right, looks like upgrade deployment tests fail with policy related to this change right?

52.18.130.203	   error: Promised replacement "    $config["cli_rest_server_url"] = "https://localhost:443/api/";" for pattern "^\s*\$config\["cli_rest_server_url"\]\s*=\s*"https://localhost.*/api/";\s*$" is not properly convergent while editing "/var/cfengine/share/GUI/application/config/config.php" (pattern still matches the end-state replacement string "    $config["cli_rest_server_url"] = "https://localhost:443/api/";", consider use of a negative look ahead)
52.18.130.203	   error: Errors encountered when actuating replace_patterns promise "^\s*\$config\["cli_rest_server_url"\]\s*=\s*"https://localhost.*/api/";\s*$"

Copy link
Contributor

@craigcomstock craigcomstock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like the policy might need some fixing for upgrade deployment tests in sequential-tests job.

@aleksandrychev
Copy link
Contributor Author

@cf-bottom jenkins

@cf-bottom
Copy link

@aleksandrychev
Copy link
Contributor Author

looks like the policy might need some fixing for upgrade deployment tests in sequential-tests job.

yes, trying config changing only if the port is changed. should help 🤞

Ticket: ENT-12151
Signed-off-by: Ihor Aleksandrychiev <[email protected]>
@aleksandrychev
Copy link
Contributor Author

@cf-bottom jenkins

@cf-bottom
Copy link

@aleksandrychev
Copy link
Contributor Author

@cf-bottom jenkins

@cf-bottom
Copy link

@aleksandrychev
Copy link
Contributor Author

@cf-bottom jenkins

@cf-bottom
Copy link

@aleksandrychev
Copy link
Contributor Author

aleksandrychev commented Oct 11, 2024

sequential-tests tests failed, I guess is not related

Errors when installing master version HUB packages:
+ grep -P "(err|fail)" .27351.install.log.filtered
52.214.238.164	check for "/var/cfengine/bin/psql" failed: cannot execute
34.244.0.54	check for "/var/cfengine/bin/psql" failed: cannot execute
34.249.48.171	check for "/var/cfengine/bin/psql" failed: cannot execute
54.75.79.92	check for "/var/cfengine/bin/psql" failed: cannot execute

restarted it

Build Status

@craigcomstock
Copy link
Contributor

craigcomstock commented Oct 11, 2024

sequential-tests tests failed, I guess is not related

Errors when installing master version HUB packages:
+ grep -P "(err|fail)" .27351.install.log.filtered
52.214.238.164	check for "/var/cfengine/bin/psql" failed: cannot execute
34.244.0.54	check for "/var/cfengine/bin/psql" failed: cannot execute
34.249.48.171	check for "/var/cfengine/bin/psql" failed: cannot execute
54.75.79.92	check for "/var/cfengine/bin/psql" failed: cannot execute

restarted it

Build Status

Correct, this is a known issue. https://northerntech.atlassian.net/browse/ENT-12383

It isn't a flake so a restart/retry won't help. We/I will fix soon.

@aleksandrychev
Copy link
Contributor Author

sequential-tests tests failed, I guess is not related

Errors when installing master version HUB packages:
+ grep -P "(err|fail)" .27351.install.log.filtered
52.214.238.164	check for "/var/cfengine/bin/psql" failed: cannot execute
34.244.0.54	check for "/var/cfengine/bin/psql" failed: cannot execute
34.249.48.171	check for "/var/cfengine/bin/psql" failed: cannot execute
54.75.79.92	check for "/var/cfengine/bin/psql" failed: cannot execute

restarted it
Build Status

Correct, this is a known issue. https://northerntech.atlassian.net/browse/ENT-12383

It isn't a flake so a restart/retry won't help. We/I will fix soon.

thanks! I will wait then

@aleksandrychev
Copy link
Contributor Author

@cf-bottom jenkins

@cf-bottom
Copy link

@aleksandrychev
Copy link
Contributor Author

@nickanderson @craigcomstock now, when tests are passed, please re-review the code. thanks!

Copy link
Member

@nickanderson nickanderson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None of my comments are blocking, but I did not actively test the policy does what is expected, just read through it logically.

cfe_internal/enterprise/CFE_hub_specific.cf Show resolved Hide resolved
cfe_internal/enterprise/mission_portal.cf Show resolved Hide resolved
@nickanderson
Copy link
Member

looks like the policy might need some fixing for upgrade deployment tests in sequential-tests job.

@craigcomstock can you please re-review?

Copy link
Contributor

@craigcomstock craigcomstock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, one more question about share vs. live, maybe de-dupe that.

CI looks good now that upgrades are mostly fixed.

cfe_internal/enterprise/CFE_hub_specific.cf Show resolved Hide resolved
@craigcomstock craigcomstock merged commit bbec246 into cfengine:master Nov 4, 2024
4 checks passed
@craigcomstock craigcomstock added the cherry-pick? Fixes which may need to be cherry-picked to LTS branches label Nov 4, 2024
@aleksandrychev aleksandrychev removed the cherry-pick? Fixes which may need to be cherry-picked to LTS branches label Nov 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants