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

[deploy][scd/crdb] Add columns uss_requested_ovn and past_ovns to scd_operations table; Bump schema_version to v3.2.0 #1095

Merged
merged 1 commit into from
Sep 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion build/db_schemas/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,6 @@ places:
schema_versions.schema_version
* [DSS main.jsonnet](../deploy/examples/minimum/main.jsonnet)
* [Schema manager main.jsonnet](../deploy/examples/schema_manager/main.jsonnet)
* scd_ or rid_ bootstrapper.sh in [dev/startup](../dev/startup)
* /pkg/{rid|scd}/store/cockroach/store.go
* /deploy/infrastructure/dependencies/terraform-commons-dss/default_latest.tf
* /deploy/services/helm-charts/dss/templates/schema-manager.yaml
2 changes: 2 additions & 0 deletions build/db_schemas/scd.libsonnet
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
"upto-v2.0.0-support_api_1_0_0.sql": importstr "scd/upto-v2.0.0-support_api_1_0_0.sql",
"upto-v3.0.0-add_inverted_indices.sql": importstr "scd/upto-v3.0.0-add_inverted_indices.sql",
"upto-v3.1.0-create_uss_availability.sql": importstr "scd/upto-v3.1.0-create_uss_availability.sql",
"upto-v3.2.0-add_ovn_columns.sql": importstr "scd/upto-v3.2.0-add_ovn_columns.sql",
"downfrom-v3.2.0-remove_ovn_columns.sql": importstr "scd/downfrom-v3.2.0-remove_ovn_columns.sql",
"downfrom-v3.1.0-remove_uss_availability.sql": importstr "scd/downfrom-v3.1.0-remove_uss_availability.sql",
"downfrom-v3.0.0-remove_inverted_indices.sql": importstr "scd/downfrom-v3.0.0-remove_inverted_indices.sql",
"downfrom-v2.0.0-remove_api_1_0_0_support.sql": importstr "scd/downfrom-v2.0.0-remove_api_1_0_0_support.sql",
Expand Down
7 changes: 7 additions & 0 deletions build/db_schemas/scd/downfrom-v3.2.0-remove_ovn_columns.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
ALTER TABLE scd_operations
DROP IF EXISTS uss_requested_ovn,
DROP IF EXISTS past_ovns;

UPDATE schema_versions
SET schema_version = 'v3.1.0'
WHERE onerow_enforcer = TRUE;
14 changes: 14 additions & 0 deletions build/db_schemas/scd/upto-v3.2.0-add_ovn_columns.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
ALTER TABLE scd_operations
ADD COLUMN IF NOT EXISTS uss_requested_ovn STRING
CHECK (uss_requested_ovn != ''), -- uss_requested_ovn must be NULL if unspecified, not an empty string
ADD COLUMN IF NOT EXISTS past_ovns STRING[] NOT NULL
DEFAULT ARRAY []::STRING[]
CHECK (
array_position(past_ovns, NULL) IS NULL AND
array_position(past_ovns, '') IS NULL AND
array_position(past_ovns, uss_requested_ovn) IS NULL
); -- past_ovns must not contain NULL elements, empty strings or current uss_requested_ovn
Comment on lines +1 to +10
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@BenjaminPelletier @barroco FTR: the only modification this data migration is doing is to add columns. Given that the queries are only ever referencing explicitly columns (as opposed to a wildcard selection), I think it is safe to conclude that previous versions of the DSS runtime are compatible with this newer version of the schema.

(follow-up to the discussion last tuesday about upgrades during the InterUSS weekly)


UPDATE schema_versions
SET schema_version = 'v3.2.0'
WHERE onerow_enforcer = TRUE;
2 changes: 1 addition & 1 deletion build/deploy/examples/minimum/main.jsonnet
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ local metadata = metadataBase {
schema_manager+: {
image: 'VAR_DOCKER_IMAGE_NAME',
desired_rid_db_version: '4.0.0',
desired_scd_db_version: '3.1.0',
desired_scd_db_version: '3.2.0',
mickmis marked this conversation as resolved.
Show resolved Hide resolved
},
prometheus+: {
storageClass: 'VAR_STORAGE_CLASS',
Expand Down
2 changes: 1 addition & 1 deletion build/deploy/examples/schema_manager/main.jsonnet
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ local metadata = metadataBase {
schema_manager+: {
image: 'VAR_DOCKER_IMAGE_NAME',
desired_rid_db_version: '4.0.0',
desired_scd_db_version: '3.1.0',
desired_scd_db_version: '3.2.0',
},
};

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
locals {
rid_db_schema = var.desired_rid_db_version == "latest" ? "4.0.0" : var.desired_rid_db_version
scd_db_schema = var.desired_scd_db_version == "latest" ? "3.1.0" : var.desired_scd_db_version
scd_db_schema = var.desired_scd_db_version == "latest" ? "3.2.0" : var.desired_scd_db_version
}
4 changes: 2 additions & 2 deletions deploy/services/helm-charts/dss/templates/schema-manager.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
{{- $jobVersion := .Release.Revision -}} {{/* Jobs template definition is immutable, using the revision in the name forces the job to be recreated at each helm upgrade. */}}
{{- $waitForCockroachDB := include "init-container-wait-for-http" (dict "serviceName" "cockroachdb" "url" (printf "http://%s:8080/health" $cockroachHost)) -}}

{{- range $service, $schemaVersion := dict "rid" "4.0.0" "scd" "3.1.0" }}
{{- range $service, $schemaVersion := dict "rid" "4.0.0" "scd" "3.2.0" }}
---
apiVersion: batch/v1
kind: Job
Expand Down Expand Up @@ -50,4 +50,4 @@ spec:
volumes:
{{- include "ca-certs:volume" . | nindent 8 }}
{{- include "client-certs:volume" . | nindent 8 }}
{{- end -}}
{{- end -}}
1 change: 1 addition & 0 deletions pkg/scd/models/operational_intents.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ type OperationalIntent struct {
Version VersionNumber
State OperationalIntentState
OVN OVN
PastOVNs []OVN
StartTime *time.Time
EndTime *time.Time
USSBaseURL string
Expand Down
49 changes: 44 additions & 5 deletions pkg/scd/store/cockroach/operational_intents.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,14 @@ import (
scdmodels "github.com/interuss/dss/pkg/scd/models"
dsssql "github.com/interuss/dss/pkg/sql"
"github.com/interuss/stacktrace"
"github.com/jackc/pgx/v5/pgtype"

"github.com/jackc/pgx/v5"
"github.com/pkg/errors"
)

var (
operationFieldsWithIndices [12]string
operationFieldsWithIndices [14]string
operationFieldsWithPrefix string
operationFieldsWithoutPrefix string
)
Expand All @@ -36,6 +37,8 @@ func init() {
operationFieldsWithIndices[9] = "updated_at"
operationFieldsWithIndices[10] = "state"
operationFieldsWithIndices[11] = "cells"
operationFieldsWithIndices[12] = "uss_requested_ovn"
operationFieldsWithIndices[13] = "past_ovns"

operationFieldsWithoutPrefix = strings.Join(
operationFieldsWithIndices[:], ",",
Expand All @@ -58,8 +61,12 @@ func (s *repo) fetchOperationalIntents(ctx context.Context, q dsssql.Queryable,
}
defer rows.Close()

var payload []*scdmodels.OperationalIntent
var cids []int64
var (
payload []*scdmodels.OperationalIntent
cids []int64
ussRequestedOVN pgtype.Text
pastOVNs []string
)
ussAvailabilities := map[dssmodels.Manager]scdmodels.UssAvailabilityState{}
for rows.Next() {
var (
Expand All @@ -79,11 +86,27 @@ func (s *repo) fetchOperationalIntents(ctx context.Context, q dsssql.Queryable,
&updatedAt,
&o.State,
&cids,
&ussRequestedOVN,
&pastOVNs,
)
if err != nil {
return nil, stacktrace.Propagate(err, "Error scanning Operation row")
}
o.OVN = scdmodels.NewOVNFromTime(updatedAt, o.ID.String())

// If the managing USS has requested a specific OVN on this operational intent, it will be persisted in DB.
// If not, a default DSS-generated OVN based on the last update time is used.
// See https://github.com/interuss/dss/issues/1078 for more details.
if ussRequestedOVN.Valid {
o.OVN = scdmodels.OVN(ussRequestedOVN.String)
} else {
o.OVN = scdmodels.NewOVNFromTime(updatedAt, o.ID.String())
}

o.PastOVNs = make([]scdmodels.OVN, 0, len(pastOVNs))
for _, pastOVN := range pastOVNs {
o.PastOVNs = append(o.PastOVNs, scdmodels.OVN(pastOVN))
}

o.SetCells(cids)
ussAvailabilities[o.Manager] = scdmodels.UssAvailabilityStateUnknown
payload = append(payload, o)
Expand Down Expand Up @@ -177,7 +200,7 @@ func (s *repo) UpsertOperationalIntent(ctx context.Context, operation *scdmodels
scd_operations
(%s)
VALUES
($1, $2, $3, $4, $5, $6, $7, $8, $9, transaction_timestamp(), $10, $11)
($1, $2, $3, $4, $5, $6, $7, $8, $9, transaction_timestamp(), $10, $11, $12, $13)
RETURNING
%s`, operationFieldsWithoutPrefix, operationFieldsWithPrefix)
)
Expand All @@ -199,6 +222,20 @@ func (s *repo) UpsertOperationalIntent(ctx context.Context, operation *scdmodels
if err != nil {
return nil, stacktrace.Propagate(err, "Failed to convert id to PgUUID")
}

var ussRequestedOVN pgtype.Text // if the OVN is to be generated by the DSS, it must be NULL in DB, not just an empty string
if operation.OVN != "" {
ussRequestedOVN = pgtype.Text{
String: operation.OVN.String(),
Valid: true,
}
}

pastOVNs := make([]string, 0, len(operation.PastOVNs))
for _, pastOVN := range operation.PastOVNs {
pastOVNs = append(pastOVNs, pastOVN.String())
}

operation, err = s.fetchOperationalIntent(ctx, s.q, upsertOperationsQuery,
opid,
operation.Manager,
Expand All @@ -211,6 +248,8 @@ func (s *repo) UpsertOperationalIntent(ctx context.Context, operation *scdmodels
subid,
operation.State,
cids,
ussRequestedOVN,
pastOVNs,
)
if err != nil {
return nil, stacktrace.Propagate(err, "Error fetching Operation")
Expand Down
Loading