Skip to content
This repository was archived by the owner on Nov 24, 2025. It is now read-only.

Interfaces first migration#4635

Merged
mattjackson220 merged 5 commits intoapache:masterfrom
ocket8888:interfaces-first-migration
May 5, 2020
Merged

Interfaces first migration#4635
mattjackson220 merged 5 commits intoapache:masterfrom
ocket8888:interfaces-first-migration

Conversation

@ocket8888
Copy link
Copy Markdown
Contributor

What does this PR (Pull Request) do?

Which Traffic Control components are affected by this PR?

  • Traffic Ops (database)

What is the best way to verify this PR?

Run a fresh installation of Traffic Ops and the database, verify that the new tables exist.

Run the migration against a database with existing data, verify that tables are properly populated and nothing old is mutated.

The following criteria are ALL met by this PR

  • We don't have database tests
  • Documentation is unnecessary
  • An update to CHANGELOG.md is not necessary
  • This PR includes any and all required license headers
  • This PR ensures that database migration sequence is correct
  • This PR DOES NOT FIX A SERIOUS SECURITY VULNERABILITY

@ocket8888 ocket8888 added new feature A new feature, capability or behavior database relating to setup/installation/structure of the Traffic Ops database low impact affects only a small portion of a CDN, and cannot itself break one labels Apr 15, 2020
Comment thread traffic_ops/app/db/migrations/20200414000000_interfaces.sql Outdated
Copy link
Copy Markdown
Member

@shamrickus shamrickus left a comment

Choose a reason for hiding this comment

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

LGTM

  • No weasel issues
  • DB can be migrated and rolled back
  • Data appears to be properly seeded from existing data.
  • Design matches BP

@ocket8888
Copy link
Copy Markdown
Contributor Author

This isn't ready to merge yet, my company has some production data that includes subnets in gateways, which under this model is not allowed. So I'm waiting to hear back about their usage and necessity.

@ocket8888
Copy link
Copy Markdown
Contributor Author

Okay, it was determined those are probably mistakes, so I added some logic to strip them on upgrades. Plus I fixed a check constraint to properly consider mask length based on address family.

Comment thread traffic_ops/app/db/migrations/20200414000000_interfaces.sql Outdated
@ocket8888 ocket8888 force-pushed the interfaces-first-migration branch from f1a2e53 to 48f0d7d Compare April 21, 2020 22:38
@ocket8888 ocket8888 force-pushed the interfaces-first-migration branch from 48f0d7d to 5bf6301 Compare April 21, 2020 22:39
@mattjackson220
Copy link
Copy Markdown
Contributor

Looks good! creates / inserts successfully, reverts successfully, old data is unchanged

@mattjackson220 mattjackson220 merged commit 8c14cc8 into apache:master May 5, 2020
@ocket8888 ocket8888 deleted the interfaces-first-migration branch May 5, 2020 14:56
ocket8888 added a commit that referenced this pull request May 7, 2020
mitchell852 pushed a commit that referenced this pull request May 7, 2020
@ocket8888 ocket8888 mentioned this pull request May 7, 2020
7 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

database relating to setup/installation/structure of the Traffic Ops database low impact affects only a small portion of a CDN, and cannot itself break one new feature A new feature, capability or behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add needed tables to TODB

3 participants