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

TES-381 Added provisioning of graphdb backup script #11

Merged
merged 1 commit into from
Jan 29, 2024
Merged

Conversation

viktor-ribchev
Copy link
Contributor

Description

Added installation of Telegraf.
Added provisioning of GraphDB backup script.
Added keepalive and file max size settings.
Improved logging of the GraphDB install script.

Related Issues

TES-381

Changes

Added installation of Telegraf.
Added provisioning of GraphDB backup script.
Added keepalive and file max size settings.
Improved logging of the GraphDB install script.

Screenshots (if applicable)

N/A

Checklist

  • I have tested these changes thoroughly.
  • My code follows the project's coding style.
  • I have added appropriate comments to my code, especially in complex areas.
  • All new and existing tests passed locally.


az login --identity

RESOURCE_GROUP="$(curl -s -H Metadata:true "http://169.254.169.254/metadata/instance/compute/resourceGroupName?api-version=2021-01-01&format=text")"
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't like these variables here. I could use the image with another user data script when creating VMs, we should not restrict this, IMO. Also, the way how the script assumes that the first storage account and app config are the ones to be used... it's wrong as it is not portable to most subscriptions. Better to pass the variables as arguments.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

@mihailradkov mihailradkov left a comment

Choose a reason for hiding this comment

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

Update the PR description to mention the extra commits

variable use_azure_cli_auth {
description = "CLI auth will use the information from an active az login session to connect to Azure and set the subscription id and tenant id associated to the signed in account."
type = bool
default = false
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's make this true by default?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


az login --identity

GRAPHDB_ADMIN_PASSWORD="$(az appconfig kv show --name ${1} --auth-mode login --key graphdb-password | jq -r .value | base64 -d)"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't we provide it? It's coupling the script to KV.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


# Checks if GraphDB is running in cluster
IS_CLUSTER=$(
curl -s -o /dev/null \
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the indent?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Added provisioning of GraphDB backup script.
Added keepalive and file max size settings.
Improved logging of the GraphDB install script.
Added script descriptions
Removed dependency on application config resource from backup script
Added support for single instance GDB to the backup script
Changed use_azure_cli_auth to true
@viktor-ribchev viktor-ribchev merged commit 3b9148e into main Jan 29, 2024
1 check passed
@viktor-ribchev viktor-ribchev deleted the TES-381 branch January 29, 2024 11:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants