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

[BUG] client update command fails #1083

Open
kanngiesser opened this issue Aug 29, 2024 · 3 comments
Open

[BUG] client update command fails #1083

kanngiesser opened this issue Aug 29, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@kanngiesser
Copy link
Contributor

kanngiesser commented Aug 29, 2024

Description

The client update command fails for service instances which were provisioned by csb client provision

Expected Behavior

It should be possible to run csb client update for service instances which were provisioned with csb client provision. Service updates should allow to change user-provided parameters and to change the service plan.

Actual Behavior

The client update command fails with error message error deciding update path: passed maintenance_info does not match the catalog maintenance_info.

Possible Fix

The client updatecommand should send a value for previous_values.maintenance_info.version and maintenance_info.version when sending the update request. Both values must match.
With the current implementation, both previous_values and maintenance_info are left empty when sending the request.

Alternatively, brokerapi/broker/decider/decider.go should be updated to match the use-case for the client update command instead of throwing ErrMaintenanceInfoConflict

Steps to Reproduce

  1. Provision a service instance: csb client provision --serviceid "${SERVICEID}" --planid "${PLANID}" --instanceid "${INSTANCEID}"
  2. Update the service instance without changing parameters: csb client update --serviceid "${SERVICEID}" --planid "${PLANID}" --instanceid "${INSTANCEID}" --instanceid "1" --params '{}'

Context

Failed to test service updates (after import) locally during Brokerpak development

Your Environment

Version used: v2.1.2
Operating System and version (desktop): Ubuntu 22.04.4 LTS (Running from Docker image mcr.microsoft.com/devcontainers/base:jammy)
Link to your project (if public): --
Platform (Azure/AWS/GCP): Local Testing
Applicable Services: Running with mariadb:11.4.2-noble backend (Docker)

@kanngiesser kanngiesser added the bug Something isn't working label Aug 29, 2024
@kanngiesser kanngiesser changed the title [BUG] client update cmmand fails [BUG] client update command fails Aug 29, 2024
@FelisiaM
Copy link
Member

FelisiaM commented Sep 2, 2024

Hi @kanngiesser

Thank you for reporting this.

The decider is doing the correct behaviour as it is conforming to the OSBAPI spec for the case when maintenance info is published.

You are correct that the right behaviour is for the client update command to send the correct maintenance info version.

Unfortunately, the team does not have the bandwidth to work on this request at the moment and we can't promise when or if we would be able to pick this up. However, we would happily review a PR if you are able to do that.

Thank you,
Felisia

@jameshochadel
Copy link
Contributor

I may have some time to contribute a fix for this. I've taken a look at the CF v3 API docs on the maintenance object. I'm curious, how is the plan version determined in the CSB? I don't see an option to specify it in the brokerpak spec.

@kanngiesser
Copy link
Contributor Author

Hi @jameshochadel, the logic which decides whether a service instance parameter should be updated or whether the service version should instead be upgraded is implemented in decider.go.

The function DecideOperation compares the maintenance information for the service plan with the data which is passed with the payload of the update request. The maintenance info for service plan appears to match the version of the OpenTofu binary which is packaged with the brokerpak, see for example registrar.go:

var maintenanceInfo *domain.MaintenanceInfo
if featureflags.Enabled(featureflags.TfUpgradeEnabled) {
  maintenanceInfo = &domain.MaintenanceInfo{
    Version:     tfBinariesContext.DefaultTfVersion.String(),
    Description: fmt.Sprintf(`This upgrade provides support for OpenTofu version: %s. The upgrade operation will take a while. The instance and all associated bindings will be upgraded.`, tfBinariesContext.DefaultTfVersion.String()),
  }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Development

No branches or pull requests

3 participants