Skip to content
This repository was archived by the owner on Sep 29, 2021. It is now read-only.

Apache reverse proxy#70

Merged
metal3-io-bot merged 1 commit into
metal3-io:masterfrom
Nordix:apache-reverse-proxy-nam
Mar 5, 2021
Merged

Apache reverse proxy#70
metal3-io-bot merged 1 commit into
metal3-io:masterfrom
Nordix:apache-reverse-proxy-nam

Conversation

@namnx228
Copy link
Copy Markdown
Member

@namnx228 namnx228 commented Nov 23, 2020 •
edited
Loading

This PR provides an option to run Ironic-inspector behind reverse proxy (Apache) and lets this Apache server takes care of TLS.

Related PRs:
metal3-io/baremetal-operator#728
metal3-io/metal3-dev-env#562

@metal3-io-bot metal3-io-bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Nov 23, 2020
This was referenced Nov 23, 2020
Merged
Closed
@namnx228
Copy link
Copy Markdown
Member Author

namnx228 commented Nov 23, 2020

/cc @kashifest
/cc @maelk
/cc @fmuyassarov
/cc @furkatgofurov7

@namnx228 namnx228 mentioned this pull request Dec 1, 2020
Merged
@namnx228 namnx228 force-pushed the apache-reverse-proxy-nam branch 5 times, most recently from c3efc66 to 7199764 Compare December 1, 2020 11:11
@namnx228 namnx228 changed the title WIP: Apache reverse proxy Apache reverse proxy Dec 1, 2020
@metal3-io-bot metal3-io-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 1, 2020
@namnx228
Copy link
Copy Markdown
Member Author

namnx228 commented Dec 1, 2020

/assign @dtantsur
/cc @maelk
/cc @kashifest
/cc @zaneb
/cc @derekhiggins

@namnx228
Copy link
Copy Markdown
Member Author

namnx228 commented Dec 1, 2020

/test-integration

@namnx228 namnx228 force-pushed the apache-reverse-proxy-nam branch from 7199764 to 592545b Compare December 1, 2020 16:00
@namnx228
Copy link
Copy Markdown
Member Author

namnx228 commented Dec 1, 2020

/test-integration

dtantsur
dtantsur suggested changes Dec 1, 2020
View reviewed changes
Comment thread runironic-inspector.sh Outdated
Comment thread inspector.conf.j2
Comment thread runironic-inspector.sh
Comment thread runironic-inspector.sh Outdated
dtantsur
dtantsur suggested changes Jan 11, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@dtantsur dtantsur 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 prefer having two containers (like in ironic) rather than two processes in one. At the very least, you'll lose half of logs.

@namnx228
Copy link
Copy Markdown
Member Author

namnx228 commented Jan 11, 2021

I'd prefer having two containers (like in ironic) rather than two processes in one. At the very least, you'll lose half of logs.

You mean to have two containers, one for Ironic-inspector, and the other for httpd? I can do that separation.
Besides, how do you think about using httpd as a reverse proxy and handle TLS for ironic-inspector like I did in this PR?

@namnx228 namnx228 force-pushed the apache-reverse-proxy-nam branch from 592545b to eadda25 Compare January 12, 2021 08:34
@metal3-io-bot metal3-io-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 13, 2021
@namnx228 namnx228 force-pushed the apache-reverse-proxy-nam branch 9 times, most recently from 932915a to 02e0812 Compare January 20, 2021 11:13
@namnx228
Copy link
Copy Markdown
Member Author

namnx228 commented Jan 21, 2021

/test-centos-integration
/test-integration

@namnx228 namnx228 force-pushed the apache-reverse-proxy-nam branch from 02e0812 to 4b1049f Compare January 21, 2021 15:09
@namnx228
Copy link
Copy Markdown
Member Author

namnx228 commented Jan 21, 2021

/test-integration

@namnx228
Copy link
Copy Markdown
Member Author

namnx228 commented Jan 21, 2021

/test-centos-integration

@namnx228 namnx228 force-pushed the apache-reverse-proxy-nam branch from 4b1049f to 7e2757d Compare January 22, 2021 09:06
@namnx228
Copy link
Copy Markdown
Member Author

namnx228 commented Jan 22, 2021

/test-integration
/test-centos-integration

dtantsur
dtantsur approved these changes Jan 25, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@dtantsur dtantsur left a comment

Choose a reason for hiding this comment

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

A food for thought. Maybe we should merge ironic-image and ironic-inspector-image in one code base, from which all containers are started. This can give us only one httpd instance (and even save some space because most of dependencies are common between ironic and inspector).

@metal3-io-bot
Copy link
Copy Markdown
Collaborator

metal3-io-bot commented Jan 25, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dtantsur, namnx228

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@metal3-io-bot metal3-io-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 25, 2021
@namnx228
Copy link
Copy Markdown
Member Author

namnx228 commented Jan 25, 2021

A food for thought. Maybe we should merge ironic-image and ironic-inspector-image in one code base, from which all containers are started. This can give us only one httpd instance (and even save some space because most of dependencies are common between ironic and inspector).

Good idea, we can discuss it in the community meeting.

@maelk
Copy link
Copy Markdown
Member

maelk commented Mar 5, 2021

/lgtm

@metal3-io-bot metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label Mar 5, 2021
@metal3-io-bot metal3-io-bot merged commit 929cf5a into metal3-io:master Mar 5, 2021
@namnx228
Copy link
Copy Markdown
Member Author

namnx228 commented Mar 5, 2021

Thanks, @dtantsur and @maelk for your comments and approvals.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@fmuyassarov fmuyassarov Awaiting requested review from fmuyassarov

@furkatgofurov7 furkatgofurov7 Awaiting requested review from furkatgofurov7

@kashifest kashifest Awaiting requested review from kashifest

@maelk maelk Awaiting requested review from maelk

@derekhiggins derekhiggins Awaiting requested review from derekhiggins

@zaneb zaneb Awaiting requested review from zaneb

1 more reviewer

@dtantsur dtantsur dtantsur approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@dtantsur dtantsur

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@namnx228 @metal3-io-bot @maelk @dtantsur