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

DTSPO-18332 - Simplify onboarding to persistent preview databases #5892

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

louisehuyton
Copy link
Contributor

@louisehuyton louisehuyton commented Dec 16, 2024

Jira link

DTSPO-18332

Change description

Simplify onboarding to persistent preview databases

Testing done

Checklist

  • commit messages are meaningful and follow good commit message guidelines
  • README and other documentation has been updated / added (if needed)
  • tests have been updated / new tests has been added (if needed)
  • Does this PR introduce a breaking change

🤖AEP PR SUMMARY🤖

md

  • Added a new file postgres-setup.sh which contains a script for setting up Postgres, creating ASO and sops-secrets, and installing Sops if not installed

@louisehuyton louisehuyton requested review from a team, cpareek, Tyler-35, JordanHoey96 and ieuanb74 and removed request for a team December 16, 2024 14:38
@louisehuyton louisehuyton requested a review from a team as a code owner December 16, 2024 14:38
version: "14"
network:
delegatedSubnetResourceReference:
armId: /subscriptions/8b6ea922-0862-443e-af15-6056e1c9b9a4/resourceGroups/cft-preview-network-rg/providers/Microsoft.Network/virtualNetworks/cft-preview-vnet/subnets/postgresql
Copy link
Contributor

Choose a reason for hiding this comment

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

cft?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops should be sds, changing now

name: ${NAMESPACE}-${ENVIRONMENT}
namespace: ${NAMESPACE}
spec:
version: "14"
Copy link
Contributor

Choose a reason for hiding this comment

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

why only version 14?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

would version 16 be more suitable @thomast1906?

privateDnsZoneArmResourceReference:
armId: /subscriptions/1baf5470-1c3e-40d3-a6f7-74bfbce4b348/resourceGroups/core-infra-intsvc-rg/providers/Microsoft.Network/privateDnsZones/privatelink.postgres.database.azure.com
EOF
) >"apps/$NAMESPACE_NAME/preview/aso/$NAMESPACE_NAME-postgres.yaml"
Copy link
Contributor

Choose a reason for hiding this comment

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

preview?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed to dev locally, will push in a sec once i correct the other above changes

Copy link
Contributor

@thomast1906 thomast1906 left a comment

Choose a reason for hiding this comment

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

Is this script for CFT? I see a number of cft related references - see coments


(
cat <<EOF
apiVersion: dbforpostgresql.azure.com/v1api20210601
Copy link
Contributor

Choose a reason for hiding this comment

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

why this and not v1?


(
cat <<EOF
apiVersion: dbforpostgresql.azure.com/v1api20210601
Copy link
Contributor

Choose a reason for hiding this comment

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

^

@thomast1906
Copy link
Contributor

Can you update/confirm the testing that has been done as part of PR? I also think its missing documentation on how to use etc.

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