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

Add enhacement for bootstrap kubelet ip #1

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

Conversation

tsorya
Copy link
Owner

@tsorya tsorya commented Jul 10, 2022

No description provided.

@tsorya tsorya force-pushed the igal/bootstrap_node_ip branch from 24df27f to 045d89a Compare July 10, 2022 11:44
@tsorya tsorya changed the title Add enhacement for bootstral kubelet ip Add enhacement for bootstrap kubelet ip Jul 10, 2022
@tsorya tsorya force-pushed the igal/bootstrap_node_ip branch 2 times, most recently from 588bc96 to 3579541 Compare July 10, 2022 11:53
enhancements/baremetal/bootstrap-kubelet-ip.md Outdated Show resolved Hide resolved
# Bootstrap External IP

## Summary
When installing a new cluster with assisted-installer, you can set machine networks that will

Choose a reason for hiding this comment

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

useing UPI or assisted-installer

Comment on lines +39 to +42
The user can work around this by modifying the bootstrap ignition file; however,
this isn't a friendly experience.

Choose a reason for hiding this comment

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

Suggested change
The user can work around this by modifying the bootstrap ignition file; however,
this isn't a friendly experience.
The user can work around this by modifying the bootstrap ignition file; however,
this isn't a friendly experience.

Don't mention it here, instead move it to alternatives

Comment on lines 32 to 33
interfaces for the cluster hosts. However, currently, you cannot configure
the bootstrap machine network using the same means.

Choose a reason for hiding this comment

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

Suggested change
interfaces for the cluster hosts. However, currently, you cannot configure
the bootstrap machine network using the same means.
interfaces for the cluster hosts. However, users cannot configure
the machine network for the bootstrap node.


## Summary
When installing a new cluster with UPI, you can set machine networks that will
be set to `networkConfig` field in the `install-config.yaml` file to configure the control plane network

Choose a reason for hiding this comment

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

Consider adding what the machine network is used for..
on the nic connected to this network OCP will build the ovs bridge and all traffic between the nodes will be on this network.

the bootstrap machine network using the same means.

## Motivation

Choose a reason for hiding this comment

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

Here you should describe the problem they had in the BZ linked above

enhancements/baremetal/bootstrap-kubelet-ip.md Outdated Show resolved Hide resolved

## Proposal

We will add a new fields, `bootstrapNodeIP` to allow for further customization of

Choose a reason for hiding this comment

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

I think this should be a subnet and not a specific IP.

Copy link
Owner Author

@tsorya tsorya Jul 10, 2022

Choose a reason for hiding this comment

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

then it will require to run nodeip-configuration, kubelet doesn't know what is subnet, it requires ip. In case of setting subnet change will be much more complicated

Copy link
Owner Author

Choose a reason for hiding this comment

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

will add it as alternative

@omertuc
Copy link

omertuc commented Jul 10, 2022

How is this related / different / affects openshift#1179 ?


## Proposal

We will add a new fields, `bootstrapNodeIP` to allow for further customization of
Copy link

Choose a reason for hiding this comment

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

New fields where?

Copy link
Owner Author

@tsorya tsorya Jul 10, 2022

Choose a reason for hiding this comment

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

How is this related / different / affects openshift#1179 ?

Bootstrap is different. Nodeip is script is not part of Bootstrap flow

Copy link
Owner Author

@tsorya tsorya Jul 10, 2022

Choose a reason for hiding this comment

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

The enhancement that you pointed to is another part of making openshift network predictable. We will need both

Copy link

@omertuc omertuc Jul 10, 2022

Choose a reason for hiding this comment

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

Bootstrap is different. Nodeip is script is not part of Bootstrap flow

But still, maybe the same configuration can be used to affect both ? (I may be way off, I don't completely understand the problem yet)

Copy link
Owner Author

Choose a reason for hiding this comment

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

We already added an option to os env for this but installer team asked for enhancement to set it in install config

## Proposal

We will add a new fields, `bootstrapNodeIP` to allow for further customization of
bootstrap host ip that should be used for kubelet configuration. We will use this field to set BootstrapNodeIP field in
Copy link

Choose a reason for hiding this comment

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

line incomplete


## Proposal

We will add a new fields, `bootstrapNodeIP` to allow for further customization of
Copy link

@omertuc omertuc Jul 10, 2022

Choose a reason for hiding this comment

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

Should bootstrapNodeIP perhaps be more generic (i.e. not just for bootstrap) and indirectly set the environment in openshift#1179 ?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Don't think so. The only other alternative is to create some script that will read install config and set kubelet ip from machine cidr. It will require such script to be part of installer and somehow be brought with bootstrap ignition. I am not sure it is worth to complicate it but let's see it on broader discussion


## Proposal

We will add a new fields, `bootstrapNodeIP` to allow for further customization of
Copy link

@omertuc omertuc Jul 10, 2022

Choose a reason for hiding this comment

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

why plural "fields"? (I assume because you want to put it in both none and metal, but it's not currently clear from the text)

### API Extensions

We will add a new field to the baremetal and None platform sections of the
`install-config.yaml` file called `bootstrapNodeIP`. This is similar to the existing
Copy link

Choose a reason for hiding this comment

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

Similar in what way? what's different about them?


## Alternatives

We can document the specific scenario, and offer the ignition-based workaround
Copy link

Choose a reason for hiding this comment

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

What is the ignition based workaround?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Override ignition

Copy link

Choose a reason for hiding this comment

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

I'm asking because it's not clear from the text what are you referring to exactly (especially because you use the word "the", it sounds like you're referring to something particular the reader should know about, but I don't know what it is)

Copy link

@omertuc omertuc Jul 10, 2022

Choose a reason for hiding this comment

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

Maybe instead of the you meant an, but regardless an example / some details would be nice

@tsorya tsorya force-pushed the igal/bootstrap_node_ip branch from 3579541 to 5ad2475 Compare July 11, 2022 10:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants