-
Notifications
You must be signed in to change notification settings - Fork 39.7k
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
Copy auth plugin to client-go repo #33334
Copy auth plugin to client-go repo #33334
Conversation
I created the PR to see if it can pass the gke smoke test. There is a twist there. |
@@ -131,7 +131,7 @@ function mvfolder { | |||
# the first rule is to convert import lines like `restclient "k8s.io/client-go/pkg/client/restclient"`, | |||
# where a package alias is the same the package name. | |||
find "${CLIENT_REPO}" -type f -name "*.go" -print0 | \ | |||
xargs -0 sed -i "s,${src_package} \"${CLIENT_REPO_FROM_SRC}/${src},${dst_package} \"${CLIENT_REPO_FROM_SRC}/${dst},g" | |||
xargs -0 sed -i "s,\<${src_package} \"${CLIENT_REPO_FROM_SRC}/${src},${dst_package} \"${CLIENT_REPO_FROM_SRC}/${dst},g" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand what this change is doing. Could you explain?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's to avoid rewriting such import:
import (
fcache "k8s.io/client-go/pkg/client/cache"
)
to
import (
fcore "k8s.io/client-go/core"
)
because when rewriting invocations, the script search for whole word, it does not rewrite the invocation from fcache
to fcore
.
find "${CLIENT_REPO}" -type f -name "*.go" -print0 | xargs -0 sed -i "s,\<${src_package}\.\([a-zA-Z]\),${dst_package}\.\1,g" | ||
fi | ||
|
||
{ grep -Rl "\"${CLIENT_REPO_FROM_SRC}/${src}" "${CLIENT_REPO}" || true ; } | while read target ; do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
read -r
so it doesn't mangle any backslashes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
# rewrite imports | ||
# the first rule is to convert import lines like `restclient "k8s.io/client-go/pkg/client/restclient"`, | ||
# where a package alias is the same the package name. | ||
sed -i "s,\<${src_package} \"${CLIENT_REPO_FROM_SRC}/${src},${dst_package} \"${CLIENT_REPO_FROM_SRC}/${dst},g" ${target} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
double quote ${target}
here and below
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
0ba3d7d
to
aba615c
Compare
Jenkins Kubemark GCE e2e failed for commit aba615cd39efa9df5695302e31031a6cffc0bba3. Full PR test history. The magic incantation to run this job again is |
Jenkins GCE e2e failed for commit aba615cd39efa9df5695302e31031a6cffc0bba3. Full PR test history. The magic incantation to run this job again is |
Jenkins GKE smoke e2e failed for commit aba615cd39efa9df5695302e31031a6cffc0bba3. Full PR test history. The magic incantation to run this job again is |
Tests failed with,
Looks like I need to update some import_known_versions.go. |
aba615c
to
cfe006b
Compare
@krousey this PR is ready. PTAL. The commit "remove special clientrepo code from main repository gcp plugin" maybe the a little mysterious. Because our e2e test uses client-go, I used to add the special initialization code for auth plugin for client-go. It's not needed anymore. Because client-go/kubernetes/clientset.go will initialize the auth plugin for client-go. Please let me know if there are other non-obvious changes. Thanks! |
rename plugin/pkg/client/auth/plugins.go package name to auth add the plugin import line in client-gen update import_known_versions for release_1_5 clientset change copy.sh
run copy.sh
cfe006b
to
a397e30
Compare
Reapplying the lgtm after squash. |
@k8s-bot test this [submit-queue is verifying that this PR is safe to merge] |
Automatic merge from submit-queue |
client-go doesn't copy the auth plugin. This causes user cannot access cluster run by GKE. User will see error "No Auth Provider found for name gcp".
This PR fixes this issue. It's marked as WIP because I'll need to rebase after #32906 gets merged. Also, the fix needs to be cherry-picked into 1.4 branch to update client-go/1.4.
This change is