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

updated patch behaviour for azureHybridBenefit property in connectedk8s #5549

Merged
merged 2 commits into from
Nov 16, 2022

Conversation

akashkeshari
Copy link
Contributor


This checklist is used to make sure that common guidelines for a pull request are followed.

Related command

General Guidelines

  • Have you run azdev style <YOUR_EXT> locally? (pip install azdev required)
  • Have you run python scripts/ci/test_index.py -q locally?

For new extensions:

About Extension Publish

There is a pipeline to automatically build, upload and publish extension wheels.
Once your pull request is merged into main branch, a new pull request will be created to update src/index.json automatically.
The precondition is to put your code inside this repository and upgrade the version in the pull request but do not modify src/index.json.

@ghost ghost added Connected Kubernetes Auto-Assign Auto assign by bot labels Nov 15, 2022
@ghost ghost requested review from zhoxing-ms, jsntcy and yonzhan November 15, 2022 05:55
@ghost ghost assigned zhoxing-ms Nov 15, 2022
@ghost ghost added this to the Nov 2022 (2022-12-06) milestone Nov 15, 2022
@ghost ghost requested a review from wangzelin007 November 15, 2022 05:55
@yonzhan
Copy link
Collaborator

yonzhan commented Nov 15, 2022

connectedk8s

Comment on lines +951 to +954
# Returning the ARM response if only AHB is being updated
arm_properties_only_ahb_set = (tags is None and distribution is None and distribution_version is None and azure_hybrid_benefit is not None)
if proxy_params_unset and auto_upgrade is None and container_log_path is None and arm_properties_only_ahb_set:
return patch_cc_response
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add a test case for this change?

Copy link
Contributor Author

@akashkeshari akashkeshari Nov 16, 2022

Choose a reason for hiding this comment

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

@zhoxing-ms , The test case for that is already covered in the test file. I have re-recorded the tests which should have covered this case

self.cmd('connectedk8s update -g akkeshar -n {name} --azure-hybrid-benefit true --kube-config {kubeconfig} --yes', checks=[

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it, thanks~

@akashkeshari akashkeshari requested review from zhoxing-ms and removed request for jsntcy, wangzelin007 and yonzhan November 16, 2022 07:41
@zhoxing-ms zhoxing-ms merged commit eefe84c into Azure:main Nov 16, 2022
@azclibot
Copy link
Collaborator

[Release] Update index.json for extension [ connectedk8s ] : https://dev.azure.com/azclitools/internal/_build/results?buildId=15283&view=results

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

Successfully merging this pull request may close these issues.

5 participants