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

e2e: Improve discovered apps infrastructure #1706

Merged
merged 2 commits into from
Dec 11, 2024

Conversation

nirs
Copy link
Member

@nirs nirs commented Dec 8, 2024

  • Add types.Deployer.IsDicovered()
  • Improve names for discovered apps

Example log:

2024-12-09T03:36:34.237+0530	INFO	disapp-deploy-rbd-busybox	dractions/discovered.go:20	Protecting workload
2024-12-09T03:36:34.375+0530	INFO	disapp-deploy-rbd-busybox	dractions/discovered.go:43	Creating drpc
2024-12-09T03:36:36.939+0530	INFO	appset-deploy-cephfs-busybox	dractions/actions.go:54	Workload running on cluster "dr2"
2024-12-09T03:36:36.939+0530	INFO	appset-deploy-cephfs-busybox	dractions/actions.go:56	Annotating placement
2024-12-09T03:36:36.999+0530	INFO	appset-deploy-rbd-busybox	dractions/actions.go:54	Workload running on cluster "dr1"
2024-12-09T03:36:36.999+0530	INFO	appset-deploy-rbd-busybox	dractions/actions.go:56	Annotating placement
2024-12-09T03:36:37.147+0530	INFO	appset-deploy-cephfs-busybox	dractions/actions.go:76	Creating drpc
2024-12-09T03:36:37.235+0530	INFO	appset-deploy-rbd-busybox	dractions/actions.go:76	Creating drpc
2024-12-09T03:36:42.043+0530	INFO	subscr-deploy-rbd-busybox	deployers/retry.go:28	Subscription phase is Propagated

e2e/util/const.go Outdated Show resolved Hide resolved
@raghavendra-talur
Copy link
Member

@nirs I agree with the overall idea. There is one typo that needs to be fixed.

I am still debating if we should have a function in the interface that returns bool for discovered or just have a function called deployerType and returns a string. However, we can merge this and make changes later if we think the other way is better.

@nirs
Copy link
Member Author

nirs commented Dec 9, 2024

@nirs I agree with the overall idea. There is one typo that needs to be fixed.

I am still debating if we should have a function in the interface that returns bool for discovered or just have a function called deployerType and returns a string. However, we can merge this and make changes later if we think the other way is better.

Since this is not an API, we can use a bool now an change this later in the unlikely case that there will be another strange deployer which is not discovered.

@nirs
Copy link
Member Author

nirs commented Dec 9, 2024

@raghavendra-talur should be ready when the e2e job completes.

nirs added 2 commits December 11, 2024 18:58
This clarifies code dispatching actions for discovered apps, eliminating
cryptic type assertions. It also eliminates the dependency on the
deployers package from dractions/actions.go.

Signed-off-by: Nir Soffer <[email protected]>
- Rename DiscoveredApps to DiscoveredApp for consistency with other
  deployers names.
- Rename deployers/discoveredapps.go to deployers/discoveredapp.go to
  match the new type name.
- Rename dractions/actionsdiscoveredapps.go to dractions/discovered.go -
  this name was too long, hard to read, and breaks log formatting.
- Rename util.RamenOpsNS to util.RamenOpsNamespace for consistency
  with other constants.

Signed-off-by: Nir Soffer <[email protected]>
@raghavendra-talur raghavendra-talur merged commit 1c58d80 into RamenDR:main Dec 11, 2024
23 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