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.

Sign up for GitHub

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

Jump to bottom

Add self-hosted upgrade tests #10485

Merged
merged 1 commit into from
Jun 21, 2022
Merged

Add self-hosted upgrade tests #10485

merged 1 commit into from
Jun 21, 2022

Conversation

nandajavarma
Copy link
Contributor

@nandajavarma nandajavarma commented Jun 6, 2022 •
edited

Description

This PR adds a new werft flag to test the upgrade of gitpod self-hosted from one version to the latest. This is intended to be used in a release PR, to see how the upgrade would work from the previous version(we can specify the version) to the latest. The setup uses the beta channel of replicated, since this is where the release ends up before going to the stable channel according to our workflow. To run the test you can add the following comment to the PR:

/werft run no-preview with-upgrade-tests fromVersion=2022.4.2

The above comment will creates a GKE cluster, install 20224.2 version of Gitpod, makes sure it is up and running, upgrades the Gitpod version to latest and confirm it has the basic functioning. This is the first 🛹 , we will add the integration tests and support for different cloud providers as the second iteration.

Apologies for the lint fixes.

Related Issue(s)

Fixes #

How to test

To test this functionality you can open a workspace from this PR and run the following command once you make sure you have core-dev access:

werft run github -j .werft/build.yaml -a no-preview=true -a with-upgrade-tests=true -a fromVersion="2022.5.0"

Release Notes

NONE

Documentation

@nandajavarma nandajavarma requested a review from a team June 6, 2022 16:26
@nandajavarma nandajavarma self-assigned this Jun 6, 2022
@nandajavarma nandajavarma marked this pull request as draft June 6, 2022 16:26
@nandajavarma nandajavarma removed their assignment Jun 6, 2022
@roboquat roboquat added the size/L label Jun 6, 2022
@github-actions github-actions bot added the team: delivery Issue belongs to the self-hosted team label Jun 6, 2022
@nandajavarma
Copy link
Contributor Author

nandajavarma commented Jun 6, 2022 •
edited by werft-gitpod-dev-com bot

/werft run --no-preview --with-upgrade-tests --from-version="2022.4.2"

👍 started the job as gitpod-build-nvn-upgrade-test.5
(with .werft/ from main)

@nandajavarma
Copy link
Contributor Author

nandajavarma commented Jun 6, 2022 •
edited by werft-gitpod-dev-com bot

/werft run --no-preview --with-upgrade-tests --from-version "2022.4.2"

👍 started the job as gitpod-build-nvn-upgrade-test.6
(with .werft/ from main)

@nandajavarma
Copy link
Contributor Author

nandajavarma commented Jun 6, 2022 •
edited by werft-gitpod-dev-com bot

/werft run no-preview with-upgrade-tests from-version="2022.4.2"

👍 started the job as gitpod-build-nvn-upgrade-test.7
(with .werft/ from main)

@nandajavarma
Copy link
Contributor Author

nandajavarma commented Jun 6, 2022 •
edited by werft-gitpod-dev-com bot

/werft run no-preview with-upgrade-tests

👍 started the job as gitpod-build-nvn-upgrade-test.8
(with .werft/ from main)

@roboquat roboquat added size/XL and removed size/L labels Jun 6, 2022
@nandajavarma
Copy link
Contributor Author

nandajavarma commented Jun 6, 2022 •
edited by werft-gitpod-dev-com bot

/werft run no-preview with-upgrade-tests

👍 started the job as gitpod-build-nvn-upgrade-test.10
(with .werft/ from main)

@nandajavarma nandajavarma force-pushed the nvn/upgrade-test branch 9 times, most recently from a58ef8c to 94e835a Compare June 7, 2022 12:04
Base automatically changed from nvn/infra-tf to main June 7, 2022 13:49
@roboquat roboquat removed the size/XL label Jun 7, 2022
@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-nvn-upgrade-test.39 because the annotations in the pull request description changed
(with .werft/ from main)

@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-nvn-upgrade-test.41 because the annotations in the pull request description changed
(with .werft/ from main)

@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-nvn-upgrade-test.43 because the annotations in the pull request description changed
(with .werft/ from main)

@nandajavarma nandajavarma force-pushed the nvn/upgrade-test branch 7 times, most recently from 8ef4ae3 to 1f86985 Compare June 14, 2022 10:50
@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-nvn-upgrade-test.52 because the annotations in the pull request description changed
(with .werft/ from main)

@nandajavarma nandajavarma marked this pull request as ready for review June 15, 2022 22:02
@nandajavarma nandajavarma force-pushed the nvn/upgrade-test branch 3 times, most recently from 16c25a9 to ed97560 Compare June 16, 2022 23:20
adrienthebo
adrienthebo reviewed Jun 17, 2022
View reviewed changes
Copy link
Contributor

@adrienthebo adrienthebo left a comment

Choose a reason for hiding this comment

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

I'm a bit new to typescript so some of my comments may be borne from that lack of experience. As a whole this looks good 👍 but I'd like to get a better understanding of the code to aid my comprehension.

.werft/installer-tests.ts Outdated Show resolved Hide resolved
.werft/jobs/build/self-hosted-upgrade-tests.ts Show resolved Hide resolved
install/tests/Makefile Outdated Show resolved Hide resolved
.werft/installer-tests.ts Outdated Show resolved Hide resolved
.werft/installer-tests.ts Outdated Show resolved Hide resolved
.werft/installer-tests.ts Outdated Show resolved Hide resolved
.werft/installer-tests.ts Outdated Show resolved Hide resolved
@roboquat roboquat added size/XXL and removed size/XL labels Jun 20, 2022
adrienthebo
adrienthebo approved these changes Jun 21, 2022
View reviewed changes
Copy link
Contributor

@adrienthebo adrienthebo left a comment

Choose a reason for hiding this comment

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

Added comments with some general thoughts, none of which are blockers. LGTM! 🚢

.werft/k3s-installer-tests.yaml
@@ -63,7 +81,25 @@ pod:
(cd .werft && yarn install && mv node_modules ..) | werft log slice prep
printf '{{ toJson . }}' > context.json

npx ts-node .werft/installer-tests.ts "STANDARD_K3S_TEST"
export DESTROY="{{ .Annotations.destroy }}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion - would it make sense to simply pass all annotations to installer-tests.ts and pick apart annotations in that file? We'll keep test setup a little bit simpler and coalesced in one location.

Non-blocker, we can address this in a future PR if desired.

.werft/k3s-installer-tests.yaml
@@ -46,6 +59,11 @@ pod:
valueFrom:
fieldRef:
fieldPath: spec.nodeName
- name: USER_TOKEN # this is for the integration tests
Copy link
Contributor

Choose a reason for hiding this comment

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

Note: this environment variable is looked up within the IDE and Workspace integration tests; we're providing it here for persistence between tests.

.werft/jobs/build/self-hosted-upgrade-tests.ts
export async function triggerUpgradeTests(werft: Werft, config: JobConfig, username: string) {
werft.phase(phases.TRIGGER_UPGRADE_TESTS, "Trigger upgrade tests on self-hosted gitpod");

if (!config.withUpgradeTests || !config.fromVersion) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Opinion (non-blocker): At present the fromVersion value is required in the upgrade test definition, so this should never be reachable. If config.withUpgradeTests is true but config.fromVersion is not set then we're likely in a condition where the test configuration itself is incorrect. In this case it might be more clear to the user if we log an error about this misconfiguration and abort, rather than skip the upgrade. WDYT?

.werft/run-sh-upgrade-tests-gke.yaml
CHANNEL="beta"
fi

npx ts-node .werft/installer-tests.ts "STANDARD_GKE_UPGRADE_TEST" ${FROM_VERSION} ${CHANNEL}
Copy link
Contributor

Choose a reason for hiding this comment

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

This is another case where we're unpacking annotations within a bash script before calling .werft/installer-tests.ts. Would this also benefit from putting the annotation parsing into installer-tests.ts?

.werft/installer-tests.ts Outdated Show resolved Hide resolved
@roboquat roboquat merged commit 9dd585b into main Jun 21, 2022
@roboquat roboquat deleted the nvn/upgrade-test branch June 21, 2022 17:59
@adrienthebo adrienthebo mentioned this pull request Jun 21, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adrienthebo adrienthebo adrienthebo approved these changes

Assignees
No one assigned
Labels
release-note-none size/XXL team: delivery Issue belongs to the self-hosted team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@nandajavarma @adrienthebo @roboquat