ddev debug migrate-database mariadb:10.11 altered config.yaml more than needed

[hanoii]

I ran this command for the first time ever:

ddev debug migrate-database mariadb:10.11

All went fine but then I noticed the changed config.yaml had more than it should.

It basically brought in all changes by the add-ons present in the project, so the updated config.yaml was the merged result of all config.*.yaml present on this project.

Expected Behavior

Only mariadb should change, along with maybe what ddev config --auto or ddev config --update does, but definitely not altering it based on other config.*.yaml files.

[rfay]

Can you be more specific about what changed please? (Note that ddev debug migrate-database is in the debug section for a reason :)

I imagine you're seeing a general bug with use of config and specific parameters.

[hanoii]

In this particular project, I only had https://github.com/hanoii/ddev-pimp-my-shell/blob/f8edb376716397863bd21dbfa2644895b2866b6d/config.pimp-my-shell.yaml, and all of that appeared on the main config.yaml file along the db change. webimage_extra_packages was even formatted as a single line array.

[rfay]

I am surprised. Thanks.

[rfay]

I can reproduce this with your exact case.

The reason is that we use GetActiveApp() in

ddev/cmd/ddev/cmd/debug-migrate-database.go

Line 22 in 66c3033

app, err := ddevapp.GetActiveApp("")

That's the full app, including config.*.yaml

Then we naively write the config back in

ddev/cmd/ddev/cmd/debug-migrate-database.go

Lines 66 to 68 in 66c3033

	app.Database.Type = typeVals[0]
	app.Database.Version = typeVals[1]
	err = app.WriteConfig()

And what writes the whole thing.

I'm not sure what the exact fix is, but in this particular case we could do app.ReadConfig("", false) and change the database type and write. That, unfortunately, would miss the possible case where the database type and version were specified in config.*.yaml

[GuySartorelli]

Is there a mechanism to identify which config file contains a given setting?

[rfay]

No, I don't think so. app.ReadConfig has the includeOverrides arg:

ddev/pkg/ddevapp/config.go

Lines 314 to 338 in 7a0c06c

	func (app *DdevApp) ReadConfig(includeOverrides bool) ([]string, error) {
	// Load base .ddev/config.yaml - original config
	err := app.LoadConfigYamlFile(app.ConfigPath)
	if err != nil {
	return []string{}, fmt.Errorf("unable to load config file %s: %v", app.ConfigPath, err)
	}
	configOverrides := []string{}
	// Load config.*.y*ml after in glob order
	if includeOverrides {
	glob := filepath.Join(filepath.Dir(app.ConfigPath), "config.*.y*ml")
	configOverrides, err = filepath.Glob(glob)
	if err != nil {
	return []string{}, err
	}
	for _, item := range configOverrides {
	err = app.mergeAdditionalConfigIntoApp(item)
	if err != nil {
	return []string{}, fmt.Errorf("unable to load config file %s: %v", item, err)
	}
	}
	}

It would be a bit of a violation for DDEV to touch a config.*.yaml though, as those aren't ever changed by DDEV directly.

[hanoii]

I'm not sure what the exact fix is, but in this particular case we could do app.ReadConfig("", false) and change the database type and write.

I think this is OK, isn't this the same that ddev config --update does?

That, unfortunately, would miss the possible case where the database type and version were specified in config.*.yaml

I think that this is also currently the case: assume a mariadb of 10.4 on config.yaml, and a mariadb of 10.6 on config.db.yaml, running ddev debug migrate-database mariadb:10.11 would write 10.11 to config.yaml, but config.db.yaml would still be 10.6 which would continue to overwrite it.

It would be a bit of a violation for DDEV to touch a config.*.yaml though, as those aren't ever changed by DDEV directly.

Agreed, and the same reasoning should apply the other way around, it shouldn't care of what config.*.yaml does. So, if a config.something.yaml set a specific db (which I think should be very very rare), then there should be a reason for doing so even though the migration will be attempted, the config.something.yaml will continue to leave the db as it expects it to. At that case is up for the user to decide what to do.

Anything further I think is overcomplicating things. We could, if it's possible, process all config.*.yaml files independently in the same way we are doing now (without the main config.yaml( and then check to see if a db is being set, and warn accordingly.

But I think it's too much for a debug command.

[rfay]

It would probably be legit for ddev debug migrate-database to app.ReadConfig(false), then change the type/version, then do its job. That seems like a modest one-line change here that is in an off-the-beaten-track place. But one has to wonder where else this conflict might exist (read with includeOverrides, then write config). It doesn't seem like with current informal policies this is an easily solvable problem. But I think it would be fine to solve it here.

[GuySartorelli]

Perhaps if the database isn't in the main config file there could be a message output to the terminal that no change was made.