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

packages: Upgrade cockroachdb to 2.0.7 #4251

Merged
merged 13 commits into from Feb 5, 2019

Conversation

mhrabovcin
Copy link
Contributor

@mhrabovcin mhrabovcin commented Jan 18, 2019

High-level description

This PR:

  • bumps cockroachdb to vesion 2.0.7
  • bumps bouncer to the latest version that is compatible with cockroachdb version 2.0.7
  • improves iam-database-restore script to do the DB rename in a single transaction

Corresponding DC/OS tickets (obligatory)

These DC/OS JIRA ticket(s) must be updated (ideally closed) in the moment this PR lands:

  • DCOS-38395 bouncer: upgrade to cockroachdb v2.0.7

Checklist for all PRs

  • Added a comprehensible changelog entry to CHANGES.md or explain why this is not a user-facing change:
  • Included a test which will fail if code is reverted but test is not. If there is no test please explain here:
  • Read the DC/OS contributing guidelines
  • Followed relevant code rules Rules for Packages and Systemd

Checklist for component/package updates:

If you are changing components or packages in DC/OS (e.g. you are bumping the sha or ref of anything underneath packages), then in addition to the above please also include:


PLEASE FILL IN THE TEMPLATE ABOVE / DO NOT REMOVE ANY SECTIONS ABOVE THIS LINE

@d2iq-mergebot
Copy link
Collaborator

This repo has @mesosphere-mergebot integration. You can perform the following commands by submitting a comment. Submit a comment with content "@mesosphere-mergebot help" to view more detailed help text and examples. Be sure the have a look at the mergebot documentation, too.

@mesosphere-mergebot override-status pr-status-check jira-url 
@mesosphere-mergebot sync  
@mesosphere-mergebot merge-it  
@mesosphere-mergebot label [Ready For Review|Holding|Ship It|Request For Comment|Work In Progress] 
@mesosphere-mergebot bump-ee  
  • PR creators can apply one of [Ready For Review|Work In Progress]. Owners can apply any label.

@d2iq-mergebot
Copy link
Collaborator

@mesosphere-mergebot bump-ee

@d2iq-mergebot
Copy link
Collaborator

Enterprise Bump PR: mesosphere/dcos-enterprise/pull/4362

@d2iq-mergebot
Copy link
Collaborator

@mesosphere-mergebot bump-ee

@d2iq-mergebot
Copy link
Collaborator

Enterprise Bump mesosphere/dcos-enterprise/pull/4362 updated.

@d2iq-mergebot
Copy link
Collaborator

@mesosphere-mergebot bump-ee

@d2iq-mergebot
Copy link
Collaborator

Enterprise Bump mesosphere/dcos-enterprise/pull/4362 updated.

@d2iq-mergebot
Copy link
Collaborator

@mesosphere-mergebot bump-ee

@d2iq-mergebot
Copy link
Collaborator

Enterprise Bump mesosphere/dcos-enterprise/pull/4362 updated.

@d2iq-mergebot
Copy link
Collaborator

@mesosphere-mergebot bump-ee

@d2iq-mergebot
Copy link
Collaborator

Enterprise Bump mesosphere/dcos-enterprise/pull/4362 updated.

@d2iq-mergebot
Copy link
Collaborator

@mesosphere-mergebot bump-ee

@d2iq-mergebot
Copy link
Collaborator

Your pull request's branch is not based on the most recent version of master. Please rebase your changes against this repo's master branch.

@d2iq-mergebot
Copy link
Collaborator

@mesosphere-mergebot bump-ee

@d2iq-mergebot
Copy link
Collaborator

Your pull request's branch is not based on the most recent version of master. Please rebase your changes against this repo's master branch.

@d2iq-mergebot
Copy link
Collaborator

@mesosphere-mergebot bump-ee

@d2iq-mergebot
Copy link
Collaborator

Your pull request's branch is not based on the most recent version of master. Please rebase your changes against this repo's master branch.

@d2iq-mergebot
Copy link
Collaborator

@mesosphere-mergebot bump-ee

@d2iq-mergebot
Copy link
Collaborator

Your pull request's branch is not based on the most recent version of master. Please rebase your changes against this repo's master branch.

@d2iq-mergebot
Copy link
Collaborator

@mesosphere-mergebot bump-ee

@d2iq-mergebot
Copy link
Collaborator

Your pull request's branch is not based on the most recent version of master. Please rebase your changes against this repo's master branch.

@d2iq-mergebot
Copy link
Collaborator

@mesosphere-mergebot bump-ee

@d2iq-mergebot
Copy link
Collaborator

Your pull request's branch is not based on the most recent version of master. Please rebase your changes against this repo's master branch.

@d2iq-mergebot
Copy link
Collaborator

@mesosphere-mergebot bump-ee

@d2iq-mergebot
Copy link
Collaborator

Your pull request's branch is not based on the most recent version of master. Please rebase your changes against this repo's master branch.

@d2iq-mergebot
Copy link
Collaborator

@mesosphere-mergebot bump-ee

@d2iq-mergebot
Copy link
Collaborator

Your pull request's branch is not based on the most recent version of master. Please rebase your changes against this repo's master branch.

@d2iq-mergebot
Copy link
Collaborator

@mesosphere-mergebot bump-ee

@d2iq-mergebot
Copy link
Collaborator

Your pull request's branch is not based on the most recent version of master. Please rebase your changes against this repo's master branch.

timaa2k
timaa2k previously approved these changes Feb 1, 2019
Copy link
Contributor

@gpaul gpaul left a comment

Choose a reason for hiding this comment

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

Mostly LGTM, left some comments.

CHANGES.md Outdated
@@ -57,6 +57,8 @@ This change also aligned the authentication architectures between DC/OS Enterpri

* Allow setting environment variables for `docker-gc` in `/var/lib/dcos/docker-gc.env`

* CockroachDB has been updated to version 2.0.7
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be folded into the existing changelog entry around oss-ing cockroachdb work / left out entirely?

"git": "git@github.com:cockroachdb/cockroach.git",
"ref": "486632793834b3fded5cf8bc87dddf0041f2297e",
"ref_origin": "v1.1.9"
"kind": "url_extract",
Copy link
Contributor

Choose a reason for hiding this comment

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

IIRC the release tarballs are prepared for more convenient building from source and are not simple tars of the git repo. I suspect this makes using releases more convenient. Martin?

subprocess.run(command, input=config_text.encode('ascii'))
log.info('Command returned')

zones = ['.default', '.liveness', '.meta']
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you know whether this was relevant for v1.1.x? We might need to backport to Enterprise DC/OS1.10, 1.11 and 1.12

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think so, otherwise dcos-checks-poststart would fail on EE clusters.

def _rename_database(oldname: str, newname: str) -> None:
def _replace_database(source: str, existing: str, backup: str) -> None:
transaction = '; '.join([
'BEGIN',
Copy link
Contributor

Choose a reason for hiding this comment

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

I doubt whther renaming a database is something that can be rolled back in a transaction.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've tested following the case and it seems that capturing DB renames in a transaction works:

./cockroach sql --insecure -e "begin; SAVEPOINT cockroach_restart; ALTER DATABASE iam RENAME TO iam_o
ld; ALTER DATABASE iam_new RENAME TO iam_old; RELEASE SAVEPOINT cockroach_restart; commit;"
Error: pq: the new database name "iam_old" already exists
Failed running "sql"

and ended up with

 show databases;
+----------+
| Database |
+----------+
| iam      |
| iam_new  |
| system   |
+----------+
(3 rows)

If a transaction wasn't working I'd expect to end up with iam_old and iam_new databases.

@@ -137,25 +147,15 @@ def recover_database(my_internal_ip: str, backup_file_path: str, db_suffix: str)
_drop_database(newdbname)
raise

# 3. Then rename the active `iam` database to `iam_old`.
# 3. In a single transaction replace original `iam` database with `iam_new`
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto. I suspect this does not work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See my reply above.

@d2iq-mergebot
Copy link
Collaborator

@mesosphere-mergebot bump-ee

1 similar comment
@mhrabovcin
Copy link
Contributor Author

@mesosphere-mergebot bump-ee

@d2iq-mergebot
Copy link
Collaborator

Enterprise Bump mesosphere/dcos-enterprise/pull/4362 updated.

Copy link
Contributor

@gpaul gpaul left a comment

Choose a reason for hiding this comment

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

LGTM

@d2iq-mergebot
Copy link
Collaborator

@mesosphere-mergebot merge-it

@d2iq-mergebot
Copy link
Collaborator

Enterprise Bump PR mesosphere/dcos-enterprise/pull/4362, and the current PR #4251 are now merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants