-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Customcert and fix for non escaping values in helm #4428
Merged
Merged
Changes from 25 commits
Commits
Show all changes
35 commits
Select commit
Hold shift + click to select a range
4b7aadf
Use userValues.txt instead of --resuse-values in update command
sikasire e8297ad
fix error msg
sikasire 64531ce
added custom cert
Anumita 75101fd
change setup.py
Anumita df27b20
Helmupdatefix (#4)
Anumita 02014ea
fix output file
sikasire 55283c9
Merge branch 'customcert' of https://github.com/Anumita/azure-cli-ext…
sikasire 33669c1
Helmupdatefix (#5)
sirireddy12 159e3bb
place values file in .azure folder
sikasire c18d3df
place values file in .azure folder
sikasire bc79fea
Merge branch 'customcert' into helmupdatefix
sirireddy12 58a545a
Helmupdatefix (#6)
sirireddy12 84c1148
place values file in .azure folder
sikasire f96aad8
Merge branch 'helmupdatefix' of https://github.com/sirireddy12/azure-…
sikasire b8b5d00
Merge branch 'customcert' into helmupdatefix
sirireddy12 f44f00d
Merge pull request #7 from sirireddy12/helmupdatefix
sirireddy12 ddf86c8
nit
sikasire 95f2f5a
Merge branch 'helmupdatefix' of https://github.com/sirireddy12/azure-…
sikasire 3e42bb3
Merge pull request #8 from sirireddy12/helmupdatefix
sirireddy12 e0d6b31
nit
sikasire b5b2730
Merge pull request #9 from sirireddy12/helmupdatefix
sirireddy12 dc6cabf
nit
sikasire 6a527f4
Merge pull request #10 from sirireddy12/helmupdatefix
sirireddy12 670ace0
setp.py and history.rst updates
sikasire b740402
Merge pull request #11 from sirireddy12/helmupdatefix
sirireddy12 86783a9
modify the way of fetching userValues.txt path
sikasire 02b176f
Merge pull request #12 from sirireddy12/helmupdatefix
sirireddy12 d3500d2
Waiting for LRO to complete before moving ahead with agent installati…
sikasire 08d489e
Merge pull request #13 from sirireddy12/helmupdatefix
sirireddy12 2252380
update history.rst
sikasire 3824c2c
Merge pull request #14 from sirireddy12/helmupdatefix
sirireddy12 82989ab
fix
sikasire cc6006f
Merge pull request #15 from sirireddy12/helmupdatefix
sirireddy12 a1f6a96
fix location variable
sikasire b4f43c4
Merge pull request #16 from sirireddy12/helmupdatefix
sirireddy12 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -777,6 +777,8 @@ def update_agents(cmd, client, resource_group_name, cluster_name, https_proxy="" | |||
|
||||
# Send cloud information to telemetry | ||||
send_cloud_telemetry(cmd) | ||||
# Fetch OS details | ||||
operating_system = platform.system() | ||||
|
||||
# Setting kubeconfig | ||||
kube_config = set_kube_config(kube_config) | ||||
|
@@ -801,7 +803,7 @@ def update_agents(cmd, client, resource_group_name, cluster_name, https_proxy="" | |||
if https_proxy == "" and http_proxy == "" and no_proxy == "" and proxy_cert == "" and not disable_proxy and not auto_upgrade: | ||||
raise RequiredArgumentMissingError(consts.No_Param_Error) | ||||
|
||||
if (https_proxy or http_proxy or no_proxy or proxy_cert) and disable_proxy: | ||||
if (https_proxy or http_proxy or no_proxy) and disable_proxy: | ||||
raise MutuallyExclusiveArgumentError(consts.EnableProxy_Conflict_Error) | ||||
|
||||
# Checking whether optional extra values file has been provided. | ||||
|
@@ -879,9 +881,34 @@ def update_agents(cmd, client, resource_group_name, cluster_name, https_proxy="" | |||
# Get Helm chart path | ||||
chart_path = utils.get_chart_path(registry_path, kube_config, kube_context, helm_client_location) | ||||
|
||||
cmd_helm_values = [helm_client_location, "get", "values", "azure-arc", "--namespace", release_namespace] | ||||
if kube_config: | ||||
cmd_helm_values.extend(["--kubeconfig", kube_config]) | ||||
if kube_context: | ||||
cmd_helm_values.extend(["--kube-context", kube_context]) | ||||
|
||||
if(operating_system == 'Windows'): | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of doing this, use python apis which are generic for all OS
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. Thanks |
||||
user_values_filepath_string = r'.azure\\userValues.txt' | ||||
elif(operating_system == 'Linux' or operating_system == 'Darwin'): | ||||
user_values_filepath_string = r'.azure/userValues.txt' | ||||
else: | ||||
telemetry.set_exception(exception='Unsupported OS', fault_type=consts.Unsupported_Fault_Type, | ||||
summary=f'{operating_system} is not supported yet') | ||||
raise ClientRequestError(f'The {operating_system} platform is not currently supported.') | ||||
user_values_location = os.path.expanduser(os.path.join('~', user_values_filepath_string)) | ||||
existing_user_values = open(user_values_location, 'w+') | ||||
response_helm_values_get = Popen(cmd_helm_values, stdout=existing_user_values, stderr=PIPE) | ||||
_, error_helm_get_values = response_helm_values_get.communicate() | ||||
if response_helm_values_get.returncode != 0: | ||||
if ('forbidden' in error_helm_get_values.decode("ascii") or 'timed out waiting for the condition' in error_helm_get_values.decode("ascii")): | ||||
telemetry.set_user_fault() | ||||
telemetry.set_exception(exception=error_helm_get_values.decode("ascii"), fault_type=consts.Get_Helm_Values_Failed, | ||||
summary='Error while doing helm get values azure-arc') | ||||
raise CLIInternalError(str.format(consts.Update_Agent_Failure, error_helm_get_values.decode("ascii"))) | ||||
|
||||
cmd_helm_upgrade = [helm_client_location, "upgrade", "azure-arc", chart_path, "--namespace", release_namespace, | ||||
"--reuse-values", | ||||
"--wait", "--output", "json"] | ||||
"-f", | ||||
user_values_location, "--wait", "--output", "json"] | ||||
if values_file_provided: | ||||
cmd_helm_upgrade.extend(["-f", values_file]) | ||||
if auto_upgrade is not None: | ||||
|
@@ -898,6 +925,7 @@ def update_agents(cmd, client, resource_group_name, cluster_name, https_proxy="" | |||
cmd_helm_upgrade.extend(["--set", "global.isProxyEnabled={}".format(False)]) | ||||
if proxy_cert: | ||||
cmd_helm_upgrade.extend(["--set-file", "global.proxyCert={}".format(proxy_cert)]) | ||||
cmd_helm_upgrade.extend(["--set", "global.isCustomCert={}".format(True)]) | ||||
if kube_config: | ||||
cmd_helm_upgrade.extend(["--kubeconfig", kube_config]) | ||||
if kube_context: | ||||
|
@@ -909,8 +937,15 @@ def update_agents(cmd, client, resource_group_name, cluster_name, https_proxy="" | |||
telemetry.set_user_fault() | ||||
telemetry.set_exception(exception=error_helm_upgrade.decode("ascii"), fault_type=consts.Install_HelmRelease_Fault_Type, | ||||
summary='Unable to install helm release') | ||||
try: | ||||
os.remove(user_values_location) | ||||
except OSError: | ||||
pass | ||||
raise CLIInternalError(str.format(consts.Update_Agent_Failure, error_helm_upgrade.decode("ascii"))) | ||||
|
||||
try: | ||||
os.remove(user_values_location) | ||||
except OSError: | ||||
pass | ||||
return str.format(consts.Update_Agent_Success, connected_cluster.name) | ||||
|
||||
|
||||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this reqd?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed it