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

make drain option configuable #725

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

Conversation

walf443
Copy link
Contributor

@walf443 walf443 commented Jun 2, 2024

fix #724

@walf443 walf443 force-pushed the drain_option_configuable branch 2 times, most recently from f41d0d4 to a9cd268 Compare June 3, 2024 10:41

gracePeriod := upgradeSettings.DrainGracePeriod
if gracePeriod == "" {
gracePeriod = "30"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I missed to changed default value.

@walf443 walf443 force-pushed the drain_option_configuable branch from a9cd268 to 768204e Compare June 3, 2024 10:45
@kke
Copy link
Contributor

kke commented Jun 3, 2024

I think the config syntax needs some further thinking.

  1. Should it be node specific or global? 🤔
    Adding node specific settings becomes quite tedious if it's something you want to set for all hosts. You can use yaml anchors, but it's not really straightforward either.
    I've been thinking if there should be some kind of "host groups" or "host classes" or something else that could be used to easily set some base values for a large number of hosts. Something like:

    metadata:
      hostgroups:
        longDrain:
          drainGracePrediod: 100d
        corpBastion:
          bastion:
            address: 10.0.0.1
            port: 8822
            user: ${USER}
    spec:
      hosts:
        - role: controller
          groups:
          - longDrain
          - corpBastion 

    Maybe there could be something like this to set something for all hosts?:

    metadata:
      hostdefaults:
        drainGracePeriod: 100d
  2. Maybe it should be a flag but there could be a k0sctl.yaml way to set flags?
    There's already --no-drain which is kind of related to these settings, so adding these new settings that are set in a different place feels slightly wrong.

    But there's also reasons to have some or all of them settable through k0sctl.yaml.

    Maybe there could be something like metadata.options as an alternate source for all flag values 🤔

@kke kke added the enhancement New feature or request label Jun 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

drain option configuable
2 participants