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

Add CPI method for native disk update #2555

Merged
merged 1 commit into from
Sep 26, 2024
Merged

Conversation

s4heid
Copy link
Contributor

@s4heid s4heid commented Aug 26, 2024

What is this change about?

Certain cloud providers offer the capability to modify disk attributes (e.g. IOPS and throughput) directly, eliminating the need to create a new disk. To accommodate this, this change introduces a new CPI method called update_disk. Previously, adjusting disk properties required creating a new disk and transferring the data, a process that could be time-consuming for larger disks. Should the CPI not support this new method, BOSH will revert to the traditional approach of disk creation.

Please provide contextual information.

What tests have you run against this PR?

  • Unit tests
  • Manual testing with a CPI that implements update_disk

I couldn't get the integration tests running on my system (yet).

How should this change be described in bosh release notes?

Enables direct update of disk properties through the Cloud Provider if supported, significantly speeding up large disk updates.

Does this PR introduce a breaking change?

No, this change is compatible with older versions.

Tag your pair, your PM, and/or team!

@MSSedusch

Certain cloud providers offer the capability to modify disk attributes like
IOPS and throughput directly, eliminating the need to create a new disk. To
accommodate this, this change introduces a new CPI method called update_disk.
Previously, adjusting disk properties required creating a new disk and
transferring the data, a process that could be time-consuming for larger disks.
Should the CPI not support this new method, BOSH will revert to the traditional
approach of disk creation.

Co-authored-by: MSSedusch <[email protected]>
@jpalermo
Copy link
Member

@s4heid, is this still in progress or is it ready for review?

@s4heid
Copy link
Contributor Author

s4heid commented Aug 29, 2024

@jpalermo, it's ready for review, except for the integration tests which I couldn't set up locally, yet.

@jpalermo
Copy link
Member

Yeah, don't worry about the integration tests. We don't know how to run those either 😄

We can let CI take care of them once it's merged and fix anything that breaks.

@s4heid s4heid marked this pull request as ready for review August 31, 2024 09:20
@jpalermo jpalermo requested review from a team, lnguyen and nader-ziada and removed request for a team September 2, 2024 15:51
@beyhan
Copy link
Member

beyhan commented Sep 19, 2024

ping @nader-ziada regarding review.

Copy link
Contributor

@nader-ziada nader-ziada left a comment

Choose a reason for hiding this comment

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

lgtm. change makes sense and tests worked for me

@jpalermo jpalermo merged commit 6e7f823 into cloudfoundry:main Sep 26, 2024
1 check passed
@aramprice
Copy link
Member

it seems like this change may be breaking integration tests:

Failures:

  1) deploy with update_disk with `enable_cpi_update_disk` true updates the disk with the iaas native update method
     Failure/Error: cloud_config['instance_groups'][0]['persistent_disk_type'] = 'disk_a'
     
     NoMethodError:
       undefined method `[]' for nil
     # ./spec/integration/deploy_with_update_disk_spec.rb:93:in `deploy_update_deploy'
     # ./spec/integration/deploy_with_update_disk_spec.rb:61:in `block (3 levels) in <top (required)>'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/example.rb:263:in `instance_exec'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/example.rb:263:in `block in run'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/example.rb:511:in `block in with_around_and_singleton_context_hooks'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/example.rb:468:in `block in with_around_example_hooks'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/hooks.rb:486:in `block in run'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/hooks.rb:624:in `run_around_example_hooks_for'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/hooks.rb:486:in `run'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/example.rb:468:in `with_around_example_hooks'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/example.rb:511:in `with_around_and_singleton_context_hooks'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/example.rb:259:in `run'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/example_group.rb:646:in `block in run_examples'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/example_group.rb:642:in `map'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/example_group.rb:642:in `run_examples'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/example_group.rb:607:in `run'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/example_group.rb:608:in `block in run'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/example_group.rb:608:in `map'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/example_group.rb:608:in `run'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/runner.rb:121:in `block (3 levels) in run_specs'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/runner.rb:121:in `map'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/runner.rb:121:in `block (2 levels) in run_specs'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/configuration.rb:2092:in `with_suite_hooks'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/runner.rb:116:in `block in run_specs'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/reporter.rb:74:in `report'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/runner.rb:115:in `run_specs'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/runner.rb:89:in `run'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/runner.rb:71:in `run'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/runner.rb:45:in `invoke'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/exe/rspec:4:in `<top (required)>'
     # /usr/local/bundle/ruby/3.3.0/bin/rspec:25:in `load'
     # /usr/local/bundle/ruby/3.3.0/bin/rspec:25:in `<top (required)>'
     # /usr/local/lib/ruby/site_ruby/3.3.0/bundler/cli/exec.rb:58:in `load'
     # /usr/local/lib/ruby/site_ruby/3.3.0/bundler/cli/exec.rb:58:in `kernel_load'
     # /usr/local/lib/ruby/site_ruby/3.3.0/bundler/cli/exec.rb:23:in `run'
     # /usr/local/lib/ruby/site_ruby/3.3.0/bundler/cli.rb:455:in `exec'
     # /usr/local/lib/ruby/site_ruby/3.3.0/bundler/vendor/thor/lib/thor/command.rb:28:in `run'
     # /usr/local/lib/ruby/site_ruby/3.3.0/bundler/vendor/thor/lib/thor/invocation.rb:127:in `invoke_command'
     # /usr/local/lib/ruby/site_ruby/3.3.0/bundler/vendor/thor/lib/thor.rb:527:in `dispatch'
     # /usr/local/lib/ruby/site_ruby/3.3.0/bundler/cli.rb:35:in `dispatch'
     # /usr/local/lib/ruby/site_ruby/3.3.0/bundler/vendor/thor/lib/thor/base.rb:584:in `start'
     # /usr/local/lib/ruby/site_ruby/3.3.0/bundler/cli.rb:29:in `start'
     # /usr/local/lib/ruby/gems/3.3.0/gems/bundler-2.5.19/exe/bundle:28:in `block in <top (required)>'
     # /usr/local/lib/ruby/site_ruby/3.3.0/bundler/friendly_errors.rb:117:in `with_friendly_errors'
     # /usr/local/lib/ruby/gems/3.3.0/gems/bundler-2.5.19/exe/bundle:20:in `<top (required)>'
     # /usr/local/bundle/bin/bundle:108:in `load'
     # /usr/local/bundle/bin/bundle:108:in `<main>'

  2) deploy with update_disk with `enable_cpi_update_disk` true when CPI does not implement update_disk handles the exception
     Failure/Error: cloud_config['instance_groups'][0]['persistent_disk_type'] = 'disk_a'
     
     NoMethodError:
       undefined method `[]' for nil
     # ./spec/integration/deploy_with_update_disk_spec.rb:93:in `deploy_update_deploy'
     # ./spec/integration/deploy_with_update_disk_spec.rb:72:in `block (4 levels) in <top (required)>'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/example.rb:263:in `instance_exec'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/example.rb:263:in `block in run'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/example.rb:511:in `block in with_around_and_singleton_context_hooks'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/example.rb:468:in `block in with_around_example_hooks'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/hooks.rb:486:in `block in run'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/hooks.rb:624:in `run_around_example_hooks_for'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/hooks.rb:486:in `run'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/example.rb:468:in `with_around_example_hooks'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/example.rb:511:in `with_around_and_singleton_context_hooks'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/example.rb:259:in `run'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/example_group.rb:646:in `block in run_examples'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/example_group.rb:642:in `map'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/example_group.rb:642:in `run_examples'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/example_group.rb:607:in `run'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/example_group.rb:608:in `block in run'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/example_group.rb:608:in `map'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/example_group.rb:608:in `run'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/example_group.rb:608:in `block in run'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/example_group.rb:608:in `map'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/example_group.rb:608:in `run'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/runner.rb:121:in `block (3 levels) in run_specs'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/runner.rb:121:in `map'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/runner.rb:121:in `block (2 levels) in run_specs'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/configuration.rb:2092:in `with_suite_hooks'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/runner.rb:116:in `block in run_specs'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/reporter.rb:74:in `report'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/runner.rb:115:in `run_specs'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/runner.rb:89:in `run'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/runner.rb:71:in `run'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/runner.rb:45:in `invoke'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/exe/rspec:4:in `<top (required)>'
     # /usr/local/bundle/ruby/3.3.0/bin/rspec:25:in `load'
     # /usr/local/bundle/ruby/3.3.0/bin/rspec:25:in `<top (required)>'
     # /usr/local/lib/ruby/site_ruby/3.3.0/bundler/cli/exec.rb:58:in `load'
     # /usr/local/lib/ruby/site_ruby/3.3.0/bundler/cli/exec.rb:58:in `kernel_load'
     # /usr/local/lib/ruby/site_ruby/3.3.0/bundler/cli/exec.rb:23:in `run'
     # /usr/local/lib/ruby/site_ruby/3.3.0/bundler/cli.rb:455:in `exec'
     # /usr/local/lib/ruby/site_ruby/3.3.0/bundler/vendor/thor/lib/thor/command.rb:28:in `run'
     # /usr/local/lib/ruby/site_ruby/3.3.0/bundler/vendor/thor/lib/thor/invocation.rb:127:in `invoke_command'
     # /usr/local/lib/ruby/site_ruby/3.3.0/bundler/vendor/thor/lib/thor.rb:527:in `dispatch'
     # /usr/local/lib/ruby/site_ruby/3.3.0/bundler/cli.rb:35:in `dispatch'
     # /usr/local/lib/ruby/site_ruby/3.3.0/bundler/vendor/thor/lib/thor/base.rb:584:in `start'
     # /usr/local/lib/ruby/site_ruby/3.3.0/bundler/cli.rb:29:in `start'
     # /usr/local/lib/ruby/gems/3.3.0/gems/bundler-2.5.19/exe/bundle:28:in `block in <top (required)>'
     # /usr/local/lib/ruby/site_ruby/3.3.0/bundler/friendly_errors.rb:117:in `with_friendly_errors'
     # /usr/local/lib/ruby/gems/3.3.0/gems/bundler-2.5.19/exe/bundle:20:in `<top (required)>'
     # /usr/local/bundle/bin/bundle:108:in `load'
     # /usr/local/bundle/bin/bundle:108:in `<main>'

  3) deploy with update_disk with `enable_cpi_update_disk` false does not use the iaas native update
     Failure/Error: cloud_config['instance_groups'][0]['persistent_disk_type'] = 'disk_a'
     
     NoMethodError:
       undefined method `[]' for nil
     # ./spec/integration/deploy_with_update_disk_spec.rb:93:in `deploy_update_deploy'
     # ./spec/integration/deploy_with_update_disk_spec.rb:84:in `block (3 levels) in <top (required)>'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/example.rb:263:in `instance_exec'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/example.rb:263:in `block in run'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/example.rb:511:in `block in with_around_and_singleton_context_hooks'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/example.rb:468:in `block in with_around_example_hooks'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/hooks.rb:486:in `block in run'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/hooks.rb:624:in `run_around_example_hooks_for'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/hooks.rb:486:in `run'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/example.rb:468:in `with_around_example_hooks'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/example.rb:511:in `with_around_and_singleton_context_hooks'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/example.rb:259:in `run'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/example_group.rb:646:in `block in run_examples'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/example_group.rb:642:in `map'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/example_group.rb:642:in `run_examples'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/example_group.rb:607:in `run'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/example_group.rb:608:in `block in run'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/example_group.rb:608:in `map'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/example_group.rb:608:in `run'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/runner.rb:121:in `block (3 levels) in run_specs'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/runner.rb:121:in `map'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/runner.rb:121:in `block (2 levels) in run_specs'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/configuration.rb:2092:in `with_suite_hooks'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/runner.rb:116:in `block in run_specs'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/reporter.rb:74:in `report'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/runner.rb:115:in `run_specs'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/runner.rb:89:in `run'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/runner.rb:71:in `run'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/lib/rspec/core/runner.rb:45:in `invoke'
     # /usr/local/bundle/ruby/3.3.0/gems/rspec-core-3.13.1/exe/rspec:4:in `<top (required)>'
     # /usr/local/bundle/ruby/3.3.0/bin/rspec:25:in `load'
     # /usr/local/bundle/ruby/3.3.0/bin/rspec:25:in `<top (required)>'
     # /usr/local/lib/ruby/site_ruby/3.3.0/bundler/cli/exec.rb:58:in `load'
     # /usr/local/lib/ruby/site_ruby/3.3.0/bundler/cli/exec.rb:58:in `kernel_load'
     # /usr/local/lib/ruby/site_ruby/3.3.0/bundler/cli/exec.rb:23:in `run'
     # /usr/local/lib/ruby/site_ruby/3.3.0/bundler/cli.rb:455:in `exec'
     # /usr/local/lib/ruby/site_ruby/3.3.0/bundler/vendor/thor/lib/thor/command.rb:28:in `run'
     # /usr/local/lib/ruby/site_ruby/3.3.0/bundler/vendor/thor/lib/thor/invocation.rb:127:in `invoke_command'
     # /usr/local/lib/ruby/site_ruby/3.3.0/bundler/vendor/thor/lib/thor.rb:527:in `dispatch'
     # /usr/local/lib/ruby/site_ruby/3.3.0/bundler/cli.rb:35:in `dispatch'
     # /usr/local/lib/ruby/site_ruby/3.3.0/bundler/vendor/thor/lib/thor/base.rb:584:in `start'
     # /usr/local/lib/ruby/site_ruby/3.3.0/bundler/cli.rb:29:in `start'
     # /usr/local/lib/ruby/gems/3.3.0/gems/bundler-2.5.19/exe/bundle:28:in `block in <top (required)>'
     # /usr/local/lib/ruby/site_ruby/3.3.0/bundler/friendly_errors.rb:117:in `with_friendly_errors'
     # /usr/local/lib/ruby/gems/3.3.0/gems/bundler-2.5.19/exe/bundle:20:in `<top (required)>'
     # /usr/local/bundle/bin/bundle:108:in `load'
     # /usr/local/bundle/bin/bundle:108:in `<main>'

Finished in 279 minutes 7 seconds (files took 3.65 seconds to load)
171 examples, 3 failures

Failed examples:

rspec ./spec/integration/deploy_with_update_disk_spec.rb:60 # deploy with update_disk with `enable_cpi_update_disk` true updates the disk with the iaas native update method
rspec ./spec/integration/deploy_with_update_disk_spec.rb:69 # deploy with update_disk with `enable_cpi_update_disk` true when CPI does not implement update_disk handles the exception
rspec ./spec/integration/deploy_with_update_disk_spec.rb:83 # deploy with update_disk with `enable_cpi_update_disk` false does not use the iaas native update

https://bosh.ci.cloudfoundry.org/teams/main/pipelines/bosh-director/jobs/integration-postgres-hotswap/builds/336#L66ebcc54:3162:3329

There are similar failures for the non-hotswap integration suites.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

6 participants