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

Bug 1572859: Rm openshift_hostname #9744

Closed
wants to merge 1 commit into from

Conversation

adellape
Copy link
Contributor

@adellape adellape commented Jun 1, 2018

xref: https://bugzilla.redhat.com/show_bug.cgi?id=1572859

@sjenning @sdodson PTAL.

Will also get a release note in via #8651.

@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jun 1, 2018
* The master host uses port *10250* to reach the nodes and does not go over SDN. It depends on the target host of the deployment and uses the computed values of `*openshift_hostname*` and `*openshift_public_hostname*`.
* The master host uses port *10250* to reach the nodes and does not go over SDN.
It depends on the target host of the deployment and uses the computed values of
`*openshift_public_hostname*`.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does this still behave the same way wrt openshift_public_hostname?

Copy link
Member

Choose a reason for hiding this comment

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

I think that neither of those variables are relevant to this particular line. The only thing that matters now is the host's hostname which is what it will use to register to the API.
@sjenning is this correct

Copy link

Choose a reason for hiding this comment

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

I believe that openshift_public_hostname is only used when generating certs (allowing a different external name to be included in the cert), not configuring the node.

Copy link
Member

Choose a reason for hiding this comment

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

It's going to reach the kubelet via the hostname that the node registers with afaik. I'd just get rid of this last bullet point.

@adellape
Copy link
Contributor Author

adellape commented Jun 1, 2018

@openshift-bot openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 23, 2018
@openshift-bot openshift-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 20, 2018
@openshift-bot openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 23, 2018
@vikram-redhat
Copy link
Contributor

@adellape - this needs to be fixed for 3.10.

@openshift-bot openshift-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 30, 2018
@vikram-redhat vikram-redhat added this to the Future Release milestone Jul 30, 2018
@adellape
Copy link
Contributor Author

Removed nodeName from node-config.yaml example as well.

@vikram-redhat
Copy link
Contributor

@adellape - make sure to CP this to the enterprise-3.11 branch now as well. I have changed the milestones and labels.

@vikram-redhat
Copy link
Contributor

@adellape - is this done?

@vikram-redhat
Copy link
Contributor

@adellape - closing this in favor of the one @kalexand-rh is now working on and that covers the same areas: #11430

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch/enterprise-3.10 branch/enterprise-3.11 size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants