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

cpu/memory configuration not reset after `crc config unset preset` #3652

Closed

cfergeau opened this issue May 11, 2023 · 0 comments

Contributor

cfergeau commented May 11, 2023

We recently added code to reset cpu/memory after a preset change, but at the moment this is only done for crc config set preset ....
These steps are not working as expected:

    crc config set preset podman
    crc config set cpus 3
    crc config unset preset
    crc config get cpus

This returns 3 instead of the expected 4 (default preset is openshift with a minimum requirement of 4 cpus).
This works as expected if I use crc config set preset okd instead of crc config unset preset.

I've started fixing this in https://github.com/cfergeau/crc/tree/config but this needs more testing (next week hopefully).

The text was updated successfully, but these errors were encountered:

cfergeau added this to Project planning: crc

cfergeau moved this to Backlog in Project planning: crc

cfergeau moved this from Backlog to In Progress in Project planning: crc

cfergeau mentioned this issue

Config: Make preset config aware of cpu/memory #3636

Merged

cfergeau added a commit to cfergeau/crc that referenced this issue


          config: Use notifier to update config on preset change

717f2a9

The RegisterNotifier API can be used to be notified when the preset
changes, and to check that the memory/cpu config values match the
requirements for the current preset.

The code is config.go is generic with respect to the config key names,
hardcoding in the generic Set() method that there will be settings named
CPUs/Preset/Memory is a layering violation. With a notifier, we can move
it back to settings.go which is where we define the settings (Preset,
CPUs, ...) we'll be using.

This will also fix crc-org#3652
as the notifiers are triggered both for Set() and Unset()

After this commit, this will work as expected:

crc config set preset podman
crc config set cpus 3
crc config unset preset
crc config get cpus

cfergeau added a commit to cfergeau/crc that referenced this issue


          config: use config notifier to call UpdateDefaults()

d08edb7

There are calls to config.UpdateDefaults in pkg/crc/api/ to reload the
configuration settings every time a config is set/unset. This ensures the
default cpus/memory values are correct when the preset changes.

Then there is code in pkg/crc/config.Set()/Unset() to reset the cpus/memory
values when the preset changes if they are out of spec.

This commit groups in UpdateDefaults all the work that is needed when
the preset changes. It moves the Set/Unset UpdateDefaults calls from
pkg/crc/api to pkg/crc/config

This should fix crc-org#3652

cfergeau added a commit to cfergeau/crc that referenced this issue


          config: Use notifier to update config on preset change

b890415

The RegisterNotifier API can be used to be notified when the preset
changes, and to check that the memory/cpu config values match the
requirements for the current preset.

The code is config.go is generic with respect to the config key names,
hardcoding in the generic Set() method that there will be settings named
CPUs/Preset/Memory is a layering violation. With a notifier, we can move
it back to settings.go which is where we define the settings (Preset,
CPUs, ...) we'll be using.

This will also fix crc-org#3652
as the notifiers are triggered both for Set() and Unset()

After this commit, this will work as expected:

crc config set preset podman
crc config set cpus 3
crc config unset preset
crc config get cpus

cfergeau added a commit to cfergeau/crc that referenced this issue


          config: use config notifier to call UpdateDefaults()

e0fa245

There are calls to config.UpdateDefaults in pkg/crc/api/ to reload the
configuration settings every time a config is set/unset. This ensures the
default cpus/memory values are correct when the preset changes.

Then there is code in pkg/crc/config.Set()/Unset() to reset the cpus/memory
values when the preset changes if they are out of spec.

This commit groups in UpdateDefaults all the work that is needed when
the preset changes. It moves the Set/Unset UpdateDefaults calls from
pkg/crc/api to pkg/crc/config

This should fix crc-org#3652

cfergeau mentioned this issue

config: Follow-ups to code resetting cpu/memory after preset change #3665

Merged

cfergeau added a commit to cfergeau/crc that referenced this issue


          config: Use notifier to update config on preset change

336f319

The RegisterNotifier API can be used to be notified when the preset
changes, and to check that the memory/cpu config values match the
requirements for the current preset.

The code is config.go is generic with respect to the config key names,
hardcoding in the generic Set() method that there will be settings named
CPUs/Preset/Memory is a layering violation. With a notifier, we can move
it back to settings.go which is where we define the settings (Preset,
CPUs, ...) we'll be using.

This will also fix crc-org#3652
as the notifiers are triggered both for Set() and Unset()

After this commit, this will work as expected:

crc config set preset podman
crc config set cpus 3
crc config unset preset
crc config get cpus

cfergeau added a commit to cfergeau/crc that referenced this issue


          config: use config notifier to call UpdateDefaults()

5fcfb61

There are calls to config.UpdateDefaults in pkg/crc/api/ to reload the
configuration settings every time a config is set/unset. This ensures the
default cpus/memory values are correct when the preset changes.

Then there is code in pkg/crc/config.Set()/Unset() to reset the cpus/memory
values when the preset changes if they are out of spec.

This commit groups in UpdateDefaults all the work that is needed when
the preset changes. It moves the Set/Unset UpdateDefaults calls from
pkg/crc/api to pkg/crc/config

This should fix crc-org#3652

cfergeau moved this from In Progress to Ready for review in Project planning: crc

cfergeau added a commit to cfergeau/crc that referenced this issue


          config: Use notifier to update config on preset change

028da23

The RegisterNotifier API can be used to be notified when the preset
changes, and to check that the memory/cpu config values match the
requirements for the current preset.

The code is config.go is generic with respect to the config key names,
hardcoding in the generic Set() method that there will be settings named
CPUs/Preset/Memory is a layering violation. With a notifier, we can move
it back to settings.go which is where we define the settings (Preset,
CPUs, ...) we'll be using.

This will also fix crc-org#3652
as the notifiers are triggered both for Set() and Unset()

After this commit, this will work as expected:

crc config set preset podman
crc config set cpus 3
crc config unset preset
crc config get cpus

cfergeau added a commit to cfergeau/crc that referenced this issue


          config: use config notifier to call UpdateDefaults()

5aa9434

There are calls to config.UpdateDefaults in pkg/crc/api/ to reload the
configuration settings every time a config is set/unset. This ensures the
default cpus/memory values are correct when the preset changes.

Then there is code in pkg/crc/config.Set()/Unset() to reset the cpus/memory
values when the preset changes if they are out of spec.

This commit groups in UpdateDefaults all the work that is needed when
the preset changes. It moves the Set/Unset UpdateDefaults calls from
pkg/crc/api to pkg/crc/config

This should fix crc-org#3652

cfergeau added a commit to cfergeau/crc that referenced this issue


          config: Use notifier to update config on preset change

71c92b1

The RegisterNotifier API can be used to be notified when the preset
changes, and to check that the memory/cpu config values match the
requirements for the current preset.

The code is config.go is generic with respect to the config key names,
hardcoding in the generic Set() method that there will be settings named
CPUs/Preset/Memory is a layering violation. With a notifier, we can move
it back to settings.go which is where we define the settings (Preset,
CPUs, ...) we'll be using.

This will also fix crc-org#3652
as the notifiers are triggered both for Set() and Unset()

After this commit, this will work as expected:

crc config set preset podman
crc config set cpus 3
crc config unset preset
crc config get cpus

cfergeau added a commit to cfergeau/crc that referenced this issue


          config: use config notifier to call UpdateDefaults()

There are calls to config.UpdateDefaults in pkg/crc/api/ to reload the
configuration settings every time a config is set/unset. This ensures the
default cpus/memory values are correct when the preset changes.

Then there is code in pkg/crc/config.Set()/Unset() to reset the cpus/memory
values when the preset changes if they are out of spec.

This commit groups in UpdateDefaults all the work that is needed when
the preset changes. It moves the Set/Unset UpdateDefaults calls from
pkg/crc/api to pkg/crc/config

This should fix crc-org#3652

cfergeau added a commit to cfergeau/crc that referenced this issue


          config: Use notifier to update config on preset change

d96c3ee

The RegisterNotifier API can be used to be notified when the preset
changes, and to check that the memory/cpu config values match the
requirements for the current preset.

The code in config.go is generic with respect to the config key names,
hardcoding in the generic Set() method that there will be settings named
CPUs/Preset/Memory is a layering violation. With a notifier, we can move
it back to settings.go which is where we define the settings (Preset,
CPUs, ...) we'll be using.

This will also fix crc-org#3652
as the notifiers are triggered both for Set() and Unset()

After this commit, this will work as expected:

crc config set preset podman
crc config set cpus 3
crc config unset preset
crc config get cpus

cfergeau added a commit to cfergeau/crc that referenced this issue


          config: use config notifier to call UpdateDefaults()

f3d33d2

There are calls to config.UpdateDefaults in pkg/crc/api/ to reload the
configuration settings every time a config is set/unset. This ensures the
default cpus/memory values are correct when the preset changes.

Then there is code in pkg/crc/config.Set()/Unset() to reset the cpus/memory
values when the preset changes if they are out of spec.

This commit groups in UpdateDefaults all the work that is needed when
the preset changes. It moves the Set/Unset UpdateDefaults calls from
pkg/crc/api to pkg/crc/config

This should fix crc-org#3652

anjannath closed this as completed in

def70fe

anjannath pushed a commit that referenced this issue


          config: use config notifier to call UpdateDefaults()

07b7902

There are calls to config.UpdateDefaults in pkg/crc/api/ to reload the
configuration settings every time a config is set/unset. This ensures the
default cpus/memory values are correct when the preset changes.

Then there is code in pkg/crc/config.Set()/Unset() to reset the cpus/memory
values when the preset changes if they are out of spec.

This commit groups in UpdateDefaults all the work that is needed when
the preset changes. It moves the Set/Unset UpdateDefaults calls from
pkg/crc/api to pkg/crc/config

This should fix #3652

github-project-automation bot moved this from Ready for review to Done in Project planning: crc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment