Add clientset support for federation e2e tests. #26953
Conversation
madhusudancs
added
area/cluster-federation
release-note-none
k8s-github-robot
added
the
size/L
| @@ -55,6 +56,8 @@ type Framework struct { | |||
| Clientset_1_2 *release_1_2.Clientset | |||
| Clientset_1_3 *release_1_3.Clientset | |||
|
|
|||
| FederationClientset *federation_internalclientset.Clientset | |||
| // TODO: remove FederationClient, all the client access must be through FederationClientset | |||
There was a problem hiding this comment.
@madhusudancs is the plan to remove FederationClient and switch over the ginkgo code to use FederationClientSet in a subsequent PR? Is there anything blocking us from doing that now?
There was a problem hiding this comment.
@madhusudancs is the plan to remove FederationClient and switch over the ginkgo code to use FederationClientSet in a subsequent PR?
@colhom yeah, that's the goal.
Is there anything blocking us from doing that now?
I don't want to change too many changes at once. First of all there is no guarantee that any of my stuff works because of the ongoing auth problems. Second, there is a time crunch at the moment and changing too many things mean a lot more debugging time.
There was a problem hiding this comment.
@madhusudancs there are only two e2e tests now, simply switching from the client to the clientset would be a handful of additional lines in the diff. imho it's easier to just do this now rather than letting the deprecated field hang around and possibly attract more dependencies or subtle interactions as federation e2e grows.
There was a problem hiding this comment.
It is definitely a line or two in the diff, but please also consider the additional debugging time required. I think getting federated service controller e2e working is the primary focus at the moment. Anything that takes even a few minutes extra without actually saving time elsewhere is out of question until that federated service e2es are merged.
k8s-github-robot
added
the
needs-rebase
|
@madhusudancs needs rebase, then LGTM. |
madhusudancs
force-pushed
the
fed-e2e-framework-clientset
branch
from
76e8c8c
to
045e4a2
Compare
madhusudancs
force-pushed
the
fed-e2e-framework-clientset
branch
from
045e4a2
to
f7486d5
Compare
|
@quinton-hoole Rebased. Adding LGTM label. |
madhusudancs
added
the
lgtm
k8s-github-robot
added
size/M
nikhiljindal
added
the
priority/important-soon
|
@nikhiljindal why this is P1? |
|
@piosz Its P1 to include it in todays 1.3 branch cut. |
|
As I wrote in #27158 (comment) there is >30 PRs in submit queue with milestone 1.3 which basically means that all of them should be included in todays 1.3 branch cut. |
|
ok lets continue the discussion there. Both of these are federation related PRs. |
|
@piosz Not all of those are blockers. |
|
@k8s-bot test this issue: #IGNORE |
|
@k8s-bot test this [submit-queue is verifying that this PR is safe to merge] |
|
GCE e2e build/test passed for commit f7486d5. |
|
Automatic merge from submit-queue |
Automatic merge from submit-queue Implement first set of federated service e2e tests. These tests are untested and there is no guarantee that they work. The ongoing auth problems is blocking these e2es from being tested and upon @quinton-hoole's request I am submitting them now. Only the last commit here needs review. Depends on #26953 cc @nikhiljindal @colhom @mfanjie @kubernetes/sig-cluster-federation
Only the last commit here needs review.
Depends on #26952.
cc @colhom @kubernetes/sig-cluster-federation