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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

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
Ticket: ENT-12151
Signed-off-by: Ihor Aleksandrychiev <[email protected]>
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.

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