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

Cra test gql to compass #15119

Merged
merged 37 commits into from Sep 21, 2022
Merged

Conversation

koala7659
Copy link
Contributor

@koala7659 koala7659 commented Aug 16, 2022

This PR contains the implementation of GraphQL client used by compass-runtime-agent component tests to connect to Compass Director API and perform various operations needed for testing scenarios:

  • Registering/Unregistering Runtime
  • Registering/Unregistering application (a remote system)
  • Registering/Unregistering Formation (scenario)
  • Assign Runtime and Application to Formation (scenario)
  • Getting one time token to connect compass-runtime-agent with compass

There is a hidden assumption here: the testing scenario always exists, and we are not adding/removing it from compass. It might be problematic

@koala7659 koala7659 marked this pull request as draft August 16, 2022 16:55
@kyma-bot kyma-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Aug 16, 2022
@netlify
Copy link

netlify bot commented Aug 19, 2022

🥰 Documentation preview ready! 🥰

Name Link
🔨 Latest commit f5c0419
🔍 Latest deploy log https://app.netlify.com/sites/kyma-project-docs-preview/deploys/632476ca935a2a000a695d60
😎 Deploy Preview https://deploy-preview-15119--kyma-project-docs-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@koala7659 koala7659 marked this pull request as ready for review August 24, 2022 07:36
@kyma-bot kyma-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 24, 2022
@mvshao mvshao added area/application-connector Issues or PRs related to application connectivity area/tests Issues or PRs related to tests labels Sep 14, 2022
@mvshao mvshao added this to the 2.8 milestone Sep 14, 2022
Copy link
Contributor

@franpog859 franpog859 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 posting these initial few comments just to speed up the process a little bit. Code review still in progress

@@ -5,7 +5,7 @@ MOCK_APP_IMAGE = "$(DOCKER_PUSH_REPOSITORY)$(DOCKER_PUSH_DIRECTORY)/mock-app:$(D

.PHONY: release image

release: publish-gateway-test publish-mock-app publish-validator-test publish-compass-runtime-agent-test
release: publish-compass-runtime-agent-test
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should revert this change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Copy link
Contributor

@franpog859 franpog859 left a comment

Choose a reason for hiding this comment

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

Some additional comments from the code review. I need to check it manually though


exec := func() error {
id, err := gs.directorClient.RegisterApplication(compassAppName)
id, err := gs.directorClient.RegisterApplication(compassAppName, scenarioName)
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't it also be assigned to the correct formation to appear in the runtime? I didn't check it manually yet, just reading the code

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'm not sure since I did not write this particular piece. We will see during tests

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok

@kyma-bot
Copy link
Contributor

kyma-bot commented Sep 19, 2022

@koala7659: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pre-kyma-components-directory-size-exporter 5b8b994 link true /test pre-kyma-components-directory-size-exporter
pre-main-kyma-integration-k3d-runtime-agent 714fec8 link false /test pre-main-kyma-integration-k3d-runtime-agent

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

Copy link
Contributor

@franpog859 franpog859 left a comment

Choose a reason for hiding this comment

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

The code looks good

@kyma-bot kyma-bot added the lgtm Looks good to me! label Sep 19, 2022
@akgalwas
Copy link
Contributor

/retest

@kyma-bot kyma-bot merged commit af1a342 into kyma-project:main Sep 21, 2022
@koala7659 koala7659 deleted the cra-test-gql-to-compass branch September 21, 2022 14:47
Abirdcfly pushed a commit to Abirdcfly/kyma that referenced this pull request Oct 5, 2022
* first approach for moving graphQL client to test application

* Adding director dependency

* extracting oauth client to new component and making it build

* continue work on graphql client implementation

* finally make build graphQL client

* update base image go version to the latest one

* minimal set of gql client ready for tests. Unit tests pass

* regenerated go mocks

* adding test configuration

* restore restart policy removed by accident

* adding RBAC to read secrets and fixes for getting tokens

* remove comments and testing app delete

* adjustments of the compass runtime agent test suite for compass director client

* adding rule to clusterRole to allow reading applications

* Addling labeling test namespace for istio injection

* Build fix

* remove label from application

* Remove application labels for scenarios, tenant optimisation, regenerate mocks, and unit tests fixes

* Reenable commented out code in Synchronization test

* Small fix for oauth clinet unit test

* go mod tidy run

* fixes after merge

* New mutation for creating application

* Adding new Director mutations: AssignApplicationToFormation, RegisterRuntime, UnregisterRuntime

* Adding new Director mutations: AssignRuntimeToFormation, GetConnectionToken

* Commented out testing code

* makefile fix

* review suggestions

* change GetConnectionToken function declaration to match second PR

* Add two new mutations Register/Unregister Formation

* Small change in argument name

* Restore old makefile content
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/application-connector Issues or PRs related to application connectivity area/tests Issues or PRs related to tests lgtm Looks good to me! size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants