[test][int][multikueue] Ignore error on config namespace multiple creation. #3146
Conversation
k8s-ci-robot
added
release-note-none
k8s-ci-robot
added
approved
✅ Deploy Preview for kubernetes-sigs-kueue canceled.
|
| @@ -213,7 +213,7 @@ func managerAndMultiKueueSetup(ctx context.Context, mgr manager.Manager, gcInter | |||
| Name: "kueue-system", | |||
| }, | |||
| } | |||
| gomega.Expect(mgr.GetClient().Create(ctx, managersConfigNamespace)).To(gomega.Succeed()) | |||
| gomega.Expect(client.IgnoreAlreadyExists(mgr.GetClient().Create(ctx, managersConfigNamespace))).To(gomega.Succeed()) | |||
There was a problem hiding this comment.
Could we just check if the kueue-system does not found in the cluster before we try to create the namespace?
There was a problem hiding this comment.
We can , but we'll need to use the APIReader of the manager since at this point the cache is not yet started, it might decrease a bit the performance... anyway.
Done
There was a problem hiding this comment.
but could we then have a race condition - that we checked and it wasn't there, but it is created a couple of ms later?
There was a problem hiding this comment.
In this particular test no, the create attempts are done once before the top level specs there is plenty of time in between.
From a generic API POV it could be, and it's a good point for attempting to create it and ignoring the "AlreadyExists" errors.
There was a problem hiding this comment.
But could we call managerAndMultiKueueSetup sequentially in this case. I suppose the time gain is negligible here, but it increases complexity. Is this changed by the recent manager integration reuse PR?
There was a problem hiding this comment.
But could we call managerAndMultiKueueSetup sequentially in this case.
The calls are far apart, One when the suite with GC is starting one when the one without GC starts.
(For ginkgo parallel runs it happens in different OS processes using different clusters ).
Is this changed by the recent manager integration reuse PR?
Yes previously the envtest "clusters" wold have bean recreated.
There was a problem hiding this comment.
Ok another option. Could we create the namespace before calling the function? It is a core namespace for kueue so sounds reasonable to be present before the manager starts
There was a problem hiding this comment.
That will be somewhere in the framework or extend the framework pass yet another init function.... it's kind of overkill since this is the only place we need it.
EDIT:
done
k8s-ci-robot
added
size/S
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
LGTM label has been added. Git tree hash: a3dcae289fc21721cde3519a40a185bedb9a7e96
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mimowo, trasc The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What type of PR is this?
/kind bug
/kind failing-test
/kind flake
What this PR does / why we need it:
[test][int][multikueue] Ignore error on config namespace multiple creation.
Ignore Already Exists error when creating the kueue config namespace.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?