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

test: add e2e test for odigos cli upgrade from latest to HEAD #1446

Merged
merged 6 commits into from
Aug 16, 2024

Conversation

blumamir
Copy link
Collaborator

@blumamir blumamir commented Aug 16, 2024

Since odigos upgrade is a potentially sensative process which can fail for various reasons, this PR adds an e2e test that starts with the latest odigos version from github, and then calls cli upgrade to the current version in HEAD.

Then it generates traffic and verify that traces are created.

  • This is a basic test for the most straight forward case. while it can catch many issues, we can consider extending it in the future to run and assert more aspects after the upgrade
  • It currently only runs cli upgrade and not helm upgrade. since we are soon going to migrate cli to helm as well, I think it can be taken care of after the refactor. @rauno56
  • I duplicated many of the existing yamls from existing tests, with slight modifications. looks like we can extract some of it to a common directory and reuse it?

@blumamir blumamir changed the title chore: add e2e test for odigos cli upgrade from latest to HEAD test: add e2e test for odigos cli upgrade from latest to HEAD Aug 16, 2024
fmt.Println("Odigos upgrade failed - unable to read the current Odigos version for migration")
os.Exit(1)
}
if !cmd.Flag("skip-version-check").Changed {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the change here is adding this new flag since the current upgrade behavior requires a valid formatted version v1.2.3 while the tests use the tag e2e-test. this flag allows the test to bypass the checks

spec:
containers:
- name: manager
image: keyval/odigos-autoscaler:e2e-test
Copy link
Collaborator Author

@blumamir blumamir Aug 16, 2024

Choose a reason for hiding this comment

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

review note: i tested it locally once with an illegal tag value keyval/odigos-autoscaler:illegal to make sure this is asserted correctly and it does

Copy link

Add e2e test for upgrade

Copy link
Contributor

@edeNFed edeNFed left a comment

Choose a reason for hiding this comment

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

looks great!

@blumamir blumamir merged commit d5c2d2f into odigos-io:main Aug 16, 2024
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants