fix: Honour machine-type from config file above the one coming from metadata server by gargnitingoogle · Pull Request #4083 · GoogleCloudPlatform/gcsfuse (original) (raw)

Summary of Changes

This pull request addresses a critical bug where the machine-type setting from the configuration file was not being respected, leading to unexpected behavior as it was being superseded by values from the metadata server. The changes introduce a robust precedence mechanism, ensuring that the machine-type is correctly identified first by CLI flags, then by the config file, and only then by the metadata server. This provides more predictable and controllable machine type identification for the application.

Highlights

• Bug Fix: Machine Type Precedence: This pull request resolves an issue where the machine-type specified in the configuration file was being incorrectly overridden by values obtained from the metadata server. The fix ensures that the machine-type is now honored according to a defined precedence.
• Precedence Order for Machine Type: A clear precedence order has been established for determining the machine-type: command-line interface (CLI) flags take highest priority, followed by the configuration file, and finally the metadata server.
• Updated getMachineType Function: The getMachineType function has been modified to accept the Config object, enabling it to correctly evaluate the machine-type from the configuration file as part of the new precedence logic.
• New Test Cases for Precedence: Comprehensive unit and integration tests have been added to validate the new machine-type precedence order, covering scenarios where the machine type is set via CLI, config file, or metadata server. Changelog
• cfg/config.go
  • Modified the call to getMachineType to pass the Config object, allowing it to consider the machine type defined in the configuration.
• cfg/optimize.go
  • Updated the getMachineType function signature to include a *Config parameter.
  • Implemented the new precedence logic: CLI flag > Config file > Metadata server for determining the machine type.
  • Updated the function's documentation to reflect the new precedence rules.
• cfg/optimize_test.go
  • Adjusted existing test calls to getMachineType to pass nil for the new Config parameter.
  • Added new test cases (TestGetMachineType_InputPrecedenceOrder1, TestGetMachineType_InputPrecedenceOrder2, TestGetMachineType_InputPrecedenceOrder3) to thoroughly verify the machine type precedence order from CLI flags, config files, and the metadata server.
• cmd/config_validation_test.go
  • Added new test cases to validate config files that explicitly set or unset the machine-type.
• cmd/root_test.go
  • Introduced a new helper function createTempConfigFile to simplify the creation of temporary configuration files for testing.
  • Added new test cases (cli_flag_overrides_config_file, config_file_overrides_metadata_server) to confirm that CLI flags correctly override config file settings, and config file settings override metadata server values for machine-type.
• cmd/testdata/set_machine_type.yaml
  • Added a new test data file to simulate a configuration file with a machine-type explicitly set.
• cmd/testdata/unset_machine_type.yaml
  • Added a new test data file representing an empty configuration for machine-type.
• tools/config-gen/templates/config.tpl
  • Modified the call to getMachineType to pass the Config object, aligning with the changes in cfg/config.go. Activity
• gargnitingoogle requested a Gemini review.
• gargnitingoogle requested a Gemini summary.
• gemini-code-assist[bot] provided a medium-priority review comment suggesting refactoring the TestGetMachineType_InputPrecedenceOrder tests into a single table-driven test for better maintainability.
• gemini-code-assist[bot] provided a medium-priority review comment suggesting moving the createTempConfigFile helper function to a top-level function for improved reusability across tests.