-
Notifications
You must be signed in to change notification settings - Fork 102
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
ENT-12151: Added possibility to configure Mission Portal web server ports #2947
Conversation
f1c9e68
to
37ef726
Compare
37ef726
to
ee4be82
Compare
There was a problem hiding this 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.
5ff1f14
to
53ec589
Compare
There was a problem hiding this 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)
@craigcomstock I adjusted distributed_cleanup.py please review |
There was a problem hiding this 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.
96b9b0c
to
53ec589
Compare
There was a problem hiding this 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.
@cf-bottom jenkins |
Sure, I triggered a build: Jenkins: https://ci.cfengine.com/job/pr-pipeline/11201/ Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-11201/ |
@cf-bottom jenkins |
Sure, I triggered a build: Jenkins: https://ci.cfengine.com/job/pr-pipeline/11219/ Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-11219/ |
right, looks like upgrade deployment tests fail with policy related to this change right?
|
There was a problem hiding this 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.
53ec589
to
126bbc0
Compare
@cf-bottom jenkins |
Sure, I triggered a build: Jenkins: https://ci.cfengine.com/job/pr-pipeline/11239/ Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-11239/ |
yes, trying config changing only if the port is changed. should help 🤞 |
126bbc0
to
b6c7096
Compare
Ticket: ENT-12151 Signed-off-by: Ihor Aleksandrychiev <[email protected]>
b6c7096
to
b91921b
Compare
@cf-bottom jenkins |
Sure, I triggered a build: Jenkins: https://ci.cfengine.com/job/pr-pipeline/11243/ Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-11243/ |
@cf-bottom jenkins |
Alright, I triggered a build: Jenkins: https://ci.cfengine.com/job/pr-pipeline/11247/ Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-11247/ |
@cf-bottom jenkins |
Alright, I triggered a build: Jenkins: https://ci.cfengine.com/job/pr-pipeline/11256/ Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-11256/ |
sequential-tests tests failed, I guess is not related
restarted it |
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 |
@cf-bottom jenkins |
Sure, I triggered a build: Jenkins: https://ci.cfengine.com/job/pr-pipeline/11301/ Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-11301/ |
@nickanderson @craigcomstock now, when tests are passed, please re-review the code. thanks! |
There was a problem hiding this 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.
@craigcomstock can you please re-review? |
There was a problem hiding this 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.
Co-authored-by: Craig Comstock <[email protected]>
Ticket: ENT-12151