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

Idempotentiate create_tables.sql#5138

Merged
rawlinp merged 10 commits intoapache:masterfrom
zrhoffman:postinstall-twice
Oct 15, 2020
Merged

Idempotentiate create_tables.sql#5138
rawlinp merged 10 commits intoapache:masterfrom
zrhoffman:postinstall-twice

Conversation

@zrhoffman
Copy link
Copy Markdown
Member

@zrhoffman zrhoffman commented Oct 12, 2020

What does this PR (Pull Request) do?

  • This PR closes Postinstall can no longer be run twice #4984
  • This PR makes it so that, even if ./admin load_schema is run more than once (for example, installing TO, with a shared database, on more than 1 host simultaneously), an SQL error should not result.

Which Traffic Control components are affected by this PR?

  • Traffic Ops (initial create_tables.sql seed data)

What is the best way to verify this PR?

  1. Run the Traffic Ops DB tests

    ./pkg traffic_ops_build;
    cd traffic_ops_db/test/docker;
    cp ../../../dist/traffic_ops-5.0.0-10961.9250b20e.el7.x86_64.rpm traffic_ops.rpm;
    docker-compose up --build;
  2. Check that the resultant schema is identical to the one we would have gotten from create_tables.sql previously.

    import_and_dump_schema() {
        output_file="$1";
        <<'DOCKER_COMMANDS' docker run --rm -i \
            -v "$(pwd)/traffic_ops/app/db/create_tables.sql:/create_tables.sql" \
            -e POSTGRES_USER=traffic_ops \
            -e POSTGRES_PASSWORD=twelve \
            -e PGPASSWORD=twelve \
            -e POSTGRES_DB=traffic_ops \
            postgres:9.6-alpine bash >"$output_file";
        trap 'echo "Error on line ${LINENO} of ${0}" >/dev/stderr; exit 1' ERR;
        set -o errexit;
        /docker-entrypoint.sh postgres >/dev/stderr &
        echo 'Waiting for Postgres to start...' >/dev/stderr;
        sleep 10;
        psql -Utraffic_ops -fcreate_tables.sql >/dev/stderr;
        pg_dump -Utraffic_ops;
    DOCKER_COMMANDS
    };
    git checkout origin/master;
    import_and_dump_schema upstream.sql;
    git checkout zrhoffman/postinstall-twice;
    import_and_dump_schema pr.sql;
    wc -l upstream.sql pr.sql; # Should be 4429 lines
    openssl sha256 upstream.sql pr.sql; # Hash should be 794fd7a23fa690eec67f6cd1257a9a8d900ac59edbdc487119ef73023a938d84

The following criteria are ALL met by this PR

  • This PR includes tests
  • You should never actually run create_tables.sql twice, no documentation necessary
  • This PR includes an update to CHANGELOG.md
  • 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 (see the Apache Software Foundation's security guidelines for details)

@zrhoffman zrhoffman added Traffic Ops related to Traffic Ops database relating to setup/installation/structure of the Traffic Ops database improvement The functionality exists but it could be improved in some way. labels Oct 12, 2020
@rawlinp
Copy link
Copy Markdown
Contributor

rawlinp commented Oct 12, 2020

This would be a good test case to add to https://github.com/apache/trafficcontrol/blob/master/traffic_ops_db/test/docker/run-db-test.sh if we can.

@zrhoffman
Copy link
Copy Markdown
Member Author

DB test added in 9250b20ec8

@zrhoffman zrhoffman requested a review from rawlinp October 12, 2020 23:30
@zrhoffman
Copy link
Copy Markdown
Member Author

Rebased to fix a merge conflict in the changelog

Copy link
Copy Markdown
Contributor

@rawlinp rawlinp left a comment

Choose a reason for hiding this comment

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

TODB tests passed

@rawlinp rawlinp merged commit cfa1d0b into apache:master Oct 15, 2020
@zrhoffman zrhoffman deleted the postinstall-twice branch October 15, 2020 21:16
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 improvement The functionality exists but it could be improved in some way. Traffic Ops related to Traffic Ops

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Postinstall can no longer be run twice

2 participants