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

Feature: add ipsec as new option to enable IPSec network #276

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

Conversation

ableischwitz
Copy link
Contributor

  • Add new option ipsec to enable IPSec during installation.
  • As OpenShift seem to make this a day2 operation, there is a additional option ipsec_only_on_day2 which does the needed change after the installation. The user will have to make sure that the MTU is adjusted to handle the additonal 50 bytes of IPSec. The ipsec option already makes sure that the MTU (default 1500) is adjusted accordingly.
  • Due to this requirement the option mtu was introduced as well (defaults to 1500).

Description

Checklist/ToDo's

  • Added documentation?
  • Added release note entry? ( docs/release-notes.md )
  • Tested?

  - As OpenShift seem to make this a day2 operation, there is a additional
    option `ipsec_only_on_day2` which does the needed change after the installation.
    The user will have to make sure that the MTU is adjusted to handle the additonal 50 bytes
    of IPSec. The `ipsec` option already makes sure that the MTU (default 1500) is adjusted
    accordingly.
  - Due to this requirement the option `mtu` was introduced as well (defaults to 1500).
@rbo
Copy link
Contributor

rbo commented May 13, 2023

/ok-to-test

@rbo
Copy link
Contributor

rbo commented Jun 12, 2023

I would suggest moving the ipsec enabled into an add-on.

To adjust the install-config.yaml, I'm plan to add a more generic approach, I don't like for every install-config.yaml setting a option/switch. Something like:

install_config_overwrite: |-
   ovnKubernetesConfig:
    mtu: 1500
    ipsecConfig: {}

and then it merges the generated config with the overwrite. What's your thoughts?

@ableischwitz
Copy link
Contributor Author

Such merge mechanism would allow quite some flexibility for the configuration, but there should also be an easy way to enable such feature. If manual adjustment would be required, we could use a default config-snippet injected into install-config.yaml, overridden by the special settings given by the user.
This would allow users to easily enable features (with opinionated defaults from the documentation) and also allow further customization. But such adjustment would be needed to be added to 'cluster.yamlfor those adjustments. It also looks like theipsec` option is now moved to a post-installation task - having it as a add-on would probably allow it to be a pre-installation or post-installation task - once there is a pre-installation hook for that.

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.

2 participants