Skip to content

Fix deprecated usage of set-output (#11844) (#11855)#11866

Merged
frouioui merged 2 commits intovitessio:release-14.0from
planetscale:dbussink/backport-release-14.0-set-output-removal
Dec 6, 2022
Merged

Fix deprecated usage of set-output (#11844) (#11855)#11866
frouioui merged 2 commits intovitessio:release-14.0from
planetscale:dbussink/backport-release-14.0-set-output-removal

Conversation

@dbussink
Copy link
Copy Markdown
Member

@dbussink dbussink commented Dec 1, 2022

The set-output command is deprecated and will break in the future. This moves to the new style for all our GitHub Actions.

See also https://github.blog/changelog/2022-10-11-github-actions-deprecating-save-state-and-set-output-commands/

Also upgrades some more runners to Ubuntu 20.04 that were missed in previous backports.

Signed-off-by: Dirkjan Bussink d.bussink@gmail.com

Checklist

  • "Backport to:" labels have been added if this change should be back-ported
  • Tests were added or are not required
  • Documentation was added or is not required

@dbussink
Fix deprecated usage of set-output (vitessio#11844) (vitessio#11855)
200c1e5 
The `set-output` command is deprecated and will break in the future.
This moves to the new style for all our GitHub Actions.

See also https://github.blog/changelog/2022-10-11-github-actions-deprecating-save-state-and-set-output-commands/

Also upgrades some more runners to Ubuntu 20.04 that were missed in
previous backports.

Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>
@vitess-bot
Copy link
Copy Markdown
Contributor

vitess-bot bot commented Dec 1, 2022 •
edited by deepthi
Loading

Review Checklist

Hello reviewers! 👋 Please follow this checklist when reviewing this Pull Request.

General

  • Ensure that the Pull Request has a descriptive title.
  • If this is a change that users need to know about, please apply the release notes (needs details) label so that merging is blocked unless the summary release notes document is included.

If a new flag is being introduced:

  • Is it really necessary to add this flag?
  • Flag names should be clear and intuitive (as far as possible)
  • Help text should be descriptive.
  • Flag names should use dashes (-) as word separators rather than underscores (_).

If a workflow is added or modified:

  • Each item in Jobs should be named in order to mark it as required.
  • If the workflow should be required, the maintainer team should be notified.

Bug fixes

  • There should be at least one unit or end-to-end test.
  • The Pull Request description should include a link to an issue that describes the bug.

Non-trivial changes

  • There should be some code comments as to why things are implemented the way they are.

New/Existing features

  • Should be documented, either by modifying the existing documentation or creating new documentation.
  • New features should have a link to a feature request issue or an RFC that documents the use cases, corner cases and test cases.

Backward compatibility

  • Protobuf changes should be wire-compatible.
  • Changes to _vt tables and RPCs need to be backward compatible.
  • vtctl command output order should be stable and awk-able.
  • RPC changes should be compatible with vitess-operator
  • If a flag is removed, then it should also be removed from VTop, if used there.

dbussink
dbussink commented Dec 1, 2022
View reviewed changes
.github/workflows/cluster_endtoend_26.yml
build:
name: Run endtoend tests on Cluster (26)
runs-on: ubuntu-18.04
runs-on: ubuntu-20.04
Copy link
Copy Markdown
Member Author

@dbussink dbussink Dec 1, 2022

Choose a reason for hiding this comment

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

@rsajwani Looks like you missed some cases in #11769, so I added them here.

@dbussink
Upgrade to Ubuntu 20.04 for endtoend tests (vitessio#11113)
d2fa0f7 
This extracts the update to Ubuntu 20.04 (and thus MySQL 8 by default)
just for the end to end tests and updates the associated tests.

Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>

Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>
@dbussink
Copy link
Copy Markdown
Member Author

dbussink commented Dec 1, 2022

Also had to include #11113 since that was missing from #11769 as well.

rsajwani
rsajwani reviewed Dec 1, 2022
View reviewed changes
.github/workflows/upgrade_downgrade_test_backups_e2e.yml
if: always() && (needs.get_previous_release.result == 'success')
name: Run Upgrade Downgrade Test
runs-on: ubuntu-18.04
runs-on: ubuntu-20.04
Copy link
Copy Markdown
Contributor

@rsajwani rsajwani Dec 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This might be delibrate.. downgraded version doesn't handled backing-up mysql8.0 correctly .. We will see if the test fails.. unless we cherry-pick #10895 in version 13 and its patches.

Copy link
Copy Markdown
Member Author

@dbussink dbussink Dec 1, 2022

Choose a reason for hiding this comment

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

@rsajwani The goal of upgrading was that we can still run CI when GitHub removes 18.04 support. So we only have 2 choices, either we upgrade or we remove the CI job. We can't leave it on 18.04 since that will stop working then.

We can also look at forcing a MySQL version earlier than 8.0.30 to fix this?

Copy link
Copy Markdown
Contributor

@rsajwani rsajwani Dec 13, 2022

Choose a reason for hiding this comment

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

@dbussink , did you check all the CI workflows were passing before pushing this PR ??
ideally we shouldn't have pushed this PR until all the test are passing. I can see Run Upgrade Downgrade Test was failing...

@frouioui frouioui merged commit a4837aa into vitessio:release-14.0 Dec 6, 2022
@frouioui frouioui deleted the dbussink/backport-release-14.0-set-output-removal branch December 6, 2022 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rsajwani rsajwani rsajwani left review comments

@frouioui frouioui frouioui approved these changes

@GuptaManan100 GuptaManan100 Awaiting requested review from GuptaManan100

@rohit-nayak-ps rohit-nayak-ps Awaiting requested review from rohit-nayak-ps rohit-nayak-ps is a code owner

@mattlord mattlord Awaiting requested review from mattlord mattlord is a code owner

@harshit-gangal harshit-gangal Awaiting requested review from harshit-gangal harshit-gangal is a code owner

@systay systay Awaiting requested review from systay systay is a code owner

+1 more reviewer

@deepthi deepthi deepthi approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Backport This is a backport Component: Build/CI Type: CI/Build

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@dbussink @deepthi @rsajwani @frouioui