-
Notifications
You must be signed in to change notification settings - Fork 82
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
[WIP] Refactoring the provider-kubeconfig.py #1326
base: master
Are you sure you want to change the base?
Conversation
d4d61ce
to
0cf0453
Compare
Looks like CI has failed: |
8d27eb2
to
553d884
Compare
https://github.com/cloud-ark/kubeplus/actions/runs/9838401881/job/27158227902?pr=1326#step:5:1901 Need to add kubernetes to requirements.txt. |
b7faeb2
to
aa17328
Compare
@devdattakulkarni I would like to propose that we move the |
aa17328
to
3dee342
Compare
Rebased with |
.github/workflows/pr.yaml
Outdated
@@ -53,7 +53,7 @@ jobs: | |||
pip3 install -r requirements.txt | |||
apiserver=`kubectl config view --minify -o jsonpath='{.clusters[0].cluster.server}'` | |||
echo "API_SERVER_URL:$apiserver" | |||
python3 provider-kubeconfig.py -s $apiserver create $KUBEPLUS_NS | |||
python3 provider-kubeconfig.py create -s $apiserver -n $KUBEPLUS_NS |
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.
About putting Namespace as a parameter under a flag - the problem is it will make Namespace an optional parameter. We want to keep Namespace a required parameter. Keeping it a positional parameter achieves this. So I suggest we keep Namespace as a positional parameter.
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.
Updated to keep namespace as a positional parameter.
@chiukapoor I have started testing the changes. Here is a gist with early results. https://gist.github.com/devdattakulkarni/bb3833c9589ca3139c971da43ac2fa62 I would suggest you try all the flags with all the commands to verify whether all combinations of commands and flags are working correctly. |
3dee342
to
e991f2f
Compare
Test 2: "Namespace does not exist" is working fine for me.
Rest of the issues are resolved in latest commit. |
* Use python kube-client * Modularize delete functionality * Add check to verify if API server is reachable * Error handling Signed-off-by: Chirayu Kapoor <[email protected]>
e991f2f
to
976738f
Compare
Rebased the PR @devdattakulkarni |
Issue
Updates
-n, --namespace
parameter for namespaceTODO
UPDATE
functionalitykubeplus/provider-kubeconfig.py
Lines 346 to 391 in d4d61ce
kubeplus/provider-kubeconfig.py
Lines 421 to 439 in d4d61ce