e2e: Easier to manage namespaces #1707
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nirs
force-pushed
the
e2e-namespace
branch
7 times, most recently
from
0b407d4 to
48bed2e
Compare
nirs
requested review from
raghavendra-talur,
rakeshgm,
BenamarMk and
asn1809
raghavendra-talur
requested changes
Dec 11, 2024
e2e/deployers/applicationset.go
Outdated
| func (a ApplicationSet) Deploy(ctx types.Context) error { | ||
| name := ctx.Name() | ||
| log := ctx.Logger() | ||
| namespace := util.ArgocdNamespace | ||
| hubNamespace := ctx.ManagementNamespace() |
There was a problem hiding this comment.
I like the names ManagementNamespace and ApplicationNamespace. I don't understand why we are calling it hubNamespace here or clusterNamespace for applicationNamespace below. Whether is the namespace is on the hub or cluster is dependent on the operation and it should be clear based on the operation. I would prefer the variable also to be called managementNamespace.
There was a problem hiding this comment.
Adding hub/cluster makes it more clear for someone which new to ramen. I can change it.
There was a problem hiding this comment.
Replaced with management/app.
e2e/test/context.go
Outdated
| @@ -14,6 +14,9 @@ import ( | |||
| "go.uber.org/zap" | |||
| ) | |||
|
|
|||
| // Make it eaisr to manage namespaces created by the tests. | |||
| const namespacePrefix = "e2e-" | |||
There was a problem hiding this comment.
s/eaisr/easier/g
s/namespacePrefix/e2eNamespacePrefix/g
Second one is optional but it makes it clear.
There was a problem hiding this comment.
Maybe appNamespacesPrefix? Since we prefix only the app namespace. e2e is clear since we are in the context of e2e/test package.
There was a problem hiding this comment.
Replaced with appNamespacePrefix
We use test.Context.Namespace() and namespace temporary variables for different namespaces which makes the code confusing and error prone. We have 2 namespaces: - management namespace: used for OCM and ramen resources, on the hub and depending on the deployer also on the managed clusters. - application namespace: use for application resources, and depending on the deployer, also for the OCM and ramen resources. In addition we typically managed resources on the hub (subscription, applicationset), but sometimes on the managed clusters (discovered apps). Changes: - Rename types.Context.Namespace() to types.Context.ManagementNamespace() - Add types.Context.AppNamespace() - When using a namespace, use the relevant context namespaces, and call temporary variables managementNamespace or appNamespace. Signed-off-by: Nir Soffer <[email protected]>
Add the management or app namespace to the initial
deploy/undeploy/protect/unprotect/failover/relocate logs.
Example logs (showing only "Deploying" logs):
2024-12-10T11:11:23.616+0200 INFO subscr-deploy-rbd-busybox
deployers/subscription.go:38 Deploying subscription in namespace
"e2e-subscr-deploy-rbd-busybox"
2024-12-10T11:11:23.616+0200 INFO appset-deploy-rbd-busybox
deployers/applicationset.go:19 Deploying applicationset in namespace "argocd"
2024-12-10T11:11:23.616+0200 INFO appset-deploy-cephfs-busybox
deployers/applicationset.go:19 Deploying applicationset in namespace "argocd"
2024-12-10T11:11:23.616+0200 INFO disapp-deploy-rbd-busybox
deployers/discoveredapps.go:30 Deploying workload in namespace
"e2e-disapp-deploy-rbd-busybox"
2024-12-10T11:11:23.616+0200 INFO subscr-deploy-cephfs-busybox
deployers/subscription.go:38 Deploying subscription in namespace
"e2e-subscr-deploy-cephfs-busybox"
Signed-off-by: Nir Soffer <[email protected]>
It it hard to manage the namespaces created for every test when they
don't have a common prefix. Add "e2e-" prefix so all of them sort
together when listing namespaces.
Example namespaces during test:
% kubectl get ns --context hub | grep e2e-
e2e-subscr-deploy-cephfs-busybox Active 6m2s
e2e-subscr-deploy-rbd-busybox Active 6m2s
% kubectl get ns --context dr1 | grep e2e-
e2e-disapp-deploy-rbd-busybox Active 4m49s
e2e-subscr-deploy-cephfs-busybox Active 4m14s
e2e-subscr-deploy-rbd-busybox Active 4m48s
% kubectl get ns --context dr2 | grep e2e-
e2e-disapp-deploy-rbd-busybox Active 5m10s
e2e-subscr-deploy-cephfs-busybox Active 5m8s
e2e-subscr-deploy-rbd-busybox Active 4m35s
Fixes: RamenDR#1611
Signed-off-by: Nir Soffer <[email protected]>
|
@raghavendra-talur should be ready when e2e job completes. |
raghavendra-talur
approved these changes
Dec 12, 2024
raghavendra-talur
approved these changes
Dec 12, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Clarify the way we use namespaces, and add a "e2e-" prefix for test namespaces.
Context includes now:
When using temporary variables, use either hubNamespace or clusterNamespace.
The application namespace has a "e2e-" prefix now.
Example namespaces during test:
Fixes #1611