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.

Sign up for GitHub

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

Jump to bottom

✨ Expose individual external URL variables #476

Merged
merged 1 commit into from
Feb 23, 2024
Merged

✨ Expose individual external URL variables #476

merged 1 commit into from
Feb 23, 2024

Conversation

lentzi90
Copy link
Member

What this PR does / why we need it:

These variables are set based on IRONIC_EXTERNAL_IP by default. In some cases it makes sense to use different IPs for them. This commit also adds these variables to the readme along with IRONIC_EXTERNAL_IP, which was previously undocumented.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

@metal3-io-bot metal3-io-bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Feb 21, 2024
@lentzi90 lentzi90 force-pushed the lentzi90/override-external-vars branch from 7dbd8b8 to 146f25f Compare February 21, 2024 13:01
@metal3-io-bot metal3-io-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Feb 21, 2024
@lentzi90
Copy link
Member Author

/test-ubuntu-integration-main
/test-centos-integration-main

@elfosardo
Copy link
Member

there are a lot of unrelated changes ot README.md
would it be possible to move them to another PR?

@lentzi90
Copy link
Member Author

there are a lot of unrelated changes ot README.md would it be possible to move them to another PR?

Possibly, but they are mostly whitespace/line-length related to please the linter. The only other thing I did in the readme was add these:

- `PROVISIONING_IP` - the specific IP to use (instead of calculating it based on
  the `PROVISIONING_INTERFACE`)
- `IRONIC_EXTERNAL_IP` - Optional external IP if Ironic is not accessible on
  `PROVISIONING_IP`.
- `IRONIC_EXTERNAL_CALLBACK_URL` - Override Ironic's external callback URL.
  Defaults to use `IRONIC_EXTERNAL_IP` if available.
- `IRONIC_EXTERNAL_HTTP_URL` - Override Ironic's external http URL. Defaults to
  use `IRONIC_EXTERNAL_IP` if available.
- `IRONIC_INSPECTOR_CALLBACK_ENDPOINT_OVERRIDE` - Override Inspector's callback
  URL. Defaults to use `IRONIC_EXTERNAL_IP` if available.

The PROVISIONING_IP is third from the top, the rest are at the bottom. PROVISIONING_IP and IRONIC_EXTERNAL_IP were already exposed variables but undocumented.

@lentzi90
Expose individual external URL variables
630006d 
These variables are set based on IRONIC_EXTERNAL_IP by default. In some
cases it makes sense to use different IPs for them.
This commit also adds these variables to the readme along with
IRONIC_EXTERNAL_IP, which was previously undocumented.

Signed-off-by: Lennart Jern <lennart.jern@est.tech>
@lentzi90 lentzi90 force-pushed the lentzi90/override-external-vars branch from 146f25f to 630006d Compare February 22, 2024 11:36
@metal3-io-bot metal3-io-bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 22, 2024
@lentzi90
Copy link
Member Author

Alright, it worked! Now without the reformatting of the readme 🙂

@lentzi90
Copy link
Member Author

/cc @elfosardo

@elfosardo
Copy link
Member

/approve

@metal3-io-bot metal3-io-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 22, 2024
@lentzi90
Copy link
Member Author

/cc @Rozzii

@lentzi90
Copy link
Member Author

/test-ubuntu-integration-main
/test-centos-integration-main

Rozzii
Rozzii approved these changes Feb 23, 2024
View reviewed changes
Copy link
Member

@Rozzii Rozzii left a comment

Choose a reason for hiding this comment

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

/lgtm
Thanks this will help with deployment customization.

@metal3-io-bot metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label Feb 23, 2024
@metal3-io-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: elfosardo, Rozzii

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

The pull request process is described here

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 merged commit e6d19ab into metal3-io:main Feb 23, 2024
9 checks passed
@metal3-io-bot metal3-io-bot deleted the lentzi90/override-external-vars branch February 23, 2024 13:32
@lentzi90 lentzi90 mentioned this pull request Apr 19, 2024
Closed
elfosardo pushed a commit to elfosardo/ironic-image that referenced this pull request Apr 29, 2024
@openshift-merge-bot
Merge pull request metal3-io#476 from elfosardo/unix-sockets-413
c4299c5 
OCPBUGS-32387: Use unix sockets by default for reverse proxy communication
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Rozzii Rozzii Rozzii approved these changes

@elfosardo elfosardo Awaiting requested review from elfosardo

Assignees
No one assigned
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/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@lentzi90 @elfosardo @metal3-io-bot @Rozzii