Skip to content
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

update deployment_crud.py to include a restart_deployment method for typed client #1452

Conversation

Priyankasaggu11929
Copy link
Contributor

@Priyankasaggu11929 Priyankasaggu11929 commented May 3, 2021

What type of PR is this?

/kind documentation
/kind cleanup

What this PR does / why we need it:

This PR updates the examples/deployment_crud.py python script, to add a rolling_restart_deployment method. This new method demonstrates how to perform a rolling restart on a deployment resource using the k8s python typed client.

The output of the script looks like:
Screenshot from 2021-05-13 13-54-53

Which issue(s) this PR fixes:

Fixes #1378

Does this PR introduce a user-facing change?

NONE

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. kind/documentation Categorizes issue or PR as related to documentation. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels May 3, 2021
@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label May 3, 2021
@Priyankasaggu11929 Priyankasaggu11929 force-pushed the psaggu-add-typed-client-deployment-rolling-restart branch from bff3fc8 to f6efc56 Compare May 3, 2021 18:58
@yliaog
Copy link
Contributor

yliaog commented May 3, 2021

thanks for the pr
/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels May 3, 2021
@Priyankasaggu11929 Priyankasaggu11929 force-pushed the psaggu-add-typed-client-deployment-rolling-restart branch from f6efc56 to b0767eb Compare May 3, 2021 19:46
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 3, 2021
@Priyankasaggu11929 Priyankasaggu11929 force-pushed the psaggu-add-typed-client-deployment-rolling-restart branch from b0767eb to 8be8e87 Compare May 3, 2021 20:02
@yliaog
Copy link
Contributor

yliaog commented May 3, 2021

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 3, 2021
@Priyankasaggu11929
Copy link
Contributor Author

Hi @yliaog, some of the travis CI jobs are failing. I’m unable to understand the issues here. Could you please point me with what might be a fix for this?

https://travis-ci.org/github/kubernetes-client/python/jobs/769364787#L329

14E691B7-AEBB-4C7A-8A24-C1CD4807AAF9

thank you.

@yliaog
Copy link
Contributor

yliaog commented May 3, 2021

from https://travis-ci.org/github/kubernetes-client/python/jobs/769364787#L329

/home/travis/build/kubernetes-client/python/kubernetes/../examples/deployment_crud.py:16:80: E501 line too long (84 > 79 characters)
/home/travis/build/kubernetes-client/python/kubernetes/../examples/deployment_crud.py:89:80: E501 line too long (81 > 79 characters)

@Priyankasaggu11929 Priyankasaggu11929 force-pushed the psaggu-add-typed-client-deployment-rolling-restart branch from 8be8e87 to c77a8ba Compare May 3, 2021 21:00
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 3, 2021
@Priyankasaggu11929 Priyankasaggu11929 force-pushed the psaggu-add-typed-client-deployment-rolling-restart branch from c77a8ba to 32558c3 Compare May 3, 2021 21:07
@Priyankasaggu11929
Copy link
Contributor Author

from https://travis-ci.org/github/kubernetes-client/python/jobs/769364787#L329

/home/travis/build/kubernetes-client/python/kubernetes/../examples/deployment_crud.py:16:80: E501 line too long (84 > 79 characters)
/home/travis/build/kubernetes-client/python/kubernetes/../examples/deployment_crud.py:89:80: E501 line too long (81 > 79 characters)

Thank you so much for pointing. I'm trying to fix them.

@Priyankasaggu11929 Priyankasaggu11929 force-pushed the psaggu-add-typed-client-deployment-rolling-restart branch 3 times, most recently from 0f7edfa to 15365fe Compare May 4, 2021 02:37
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 4, 2021
@Priyankasaggu11929 Priyankasaggu11929 force-pushed the psaggu-add-typed-client-deployment-rolling-restart branch from 15365fe to 1118e3c Compare May 4, 2021 03:34
@yliaog
Copy link
Contributor

yliaog commented May 4, 2021

closing to trigger CI
/close

@k8s-ci-robot
Copy link
Contributor

@yliaog: Reopened this PR.

In response to this:

/reopen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot reopened this May 11, 2021
# Create a deployment object with client-python API. The deployment we
# created is same as the `nginx-deployment.yaml` in the /examples folder.
deployment = create_deployment_object()
# update `spec.template.metadata` section
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to fix the pycodestyle test failure, please add an empty line above

resp.metadata.generation,
resp.spec.template.metadata.annotations,
)
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to fix the pycodestyle test failure, please remove the trailing spaces in this line.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@roycaihw, I fixed the whitespaces issues but the job still remain failing.

Another thing to note:

Just to figure out what exactly amongst the new changes I added, is a breaking change. I started by adding just the following import commands (on top of the example file in master branch)

+ import datetime
+ import pytz
   from kubernetes import client, config

The job is still failing. https://travis-ci.org/github/kubernetes-client/python/jobs/770894221

I'm not sure if I'm missing something really simple here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pycodestyle is asking for some empty lines between the imports. Sorry the tool has a strong opinion about standard v.s. third-party imports-- imports in different categories need to be separated by empty lines.

diff --git a/examples/deployment_crud.py b/examples/deployment_crud.py
index 4fd02a7..7dcd6b3 100644
--- a/examples/deployment_crud.py
+++ b/examples/deployment_crud.py
@@ -17,7 +17,9 @@ Creates, updates, and deletes a deployment using AppsV1Api.
 """
 
 import datetime
+
 import pytz
+
 from kubernetes import client, config
 
 DEPLOYMENT_NAME = "nginx-deployment"
ERROR: InvocationError for command /home/travis/build/kubernetes-client/python/scripts/update-pycodestyle.sh (exited with code 1)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@roycaihw, @yliaog, finally everything is green now 😄

Sorry, it caused a lot of trouble due to repeated failing tests (& me not understanding what exactly it required). But, I definitely learnt a lot about the tool.

Thank you so much.

@k8s-ci-robot k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 12, 2021
@Priyankasaggu11929 Priyankasaggu11929 force-pushed the psaggu-add-typed-client-deployment-rolling-restart branch from 05e2b4c to 5a55826 Compare May 12, 2021 08:54
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels May 12, 2021
@Priyankasaggu11929 Priyankasaggu11929 force-pushed the psaggu-add-typed-client-deployment-rolling-restart branch from 5a55826 to bc7a7c7 Compare May 12, 2021 09:47
@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label May 12, 2021
@Priyankasaggu11929 Priyankasaggu11929 force-pushed the psaggu-add-typed-client-deployment-rolling-restart branch from bc7a7c7 to e013888 Compare May 12, 2021 09:55
@k8s-ci-robot k8s-ci-robot removed the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 12, 2021
@Priyankasaggu11929 Priyankasaggu11929 force-pushed the psaggu-add-typed-client-deployment-rolling-restart branch 2 times, most recently from 378983a to 5fd910e Compare May 13, 2021 06:04
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels May 13, 2021
@Priyankasaggu11929 Priyankasaggu11929 force-pushed the psaggu-add-typed-client-deployment-rolling-restart branch 3 times, most recently from 483c5ae to cbc6dc3 Compare May 13, 2021 07:40
Signed-off-by: Priyanka Saggu <priyankasaggu11929@gmail.com>
@Priyankasaggu11929 Priyankasaggu11929 force-pushed the psaggu-add-typed-client-deployment-rolling-restart branch from cbc6dc3 to 525f287 Compare May 13, 2021 07:53
@Priyankasaggu11929 Priyankasaggu11929 changed the title [WIP] update deployment_crud.py to include a rolling_restart_deployment method for typed client update deployment_crud.py to include a restart_deployment method for typed client May 13, 2021
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 13, 2021
@yliaog
Copy link
Contributor

yliaog commented May 13, 2021

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 13, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Priyankasaggu11929, yliaog

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit 476ce50 into kubernetes-client:master May 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. kind/documentation Categorizes issue or PR as related to documentation. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How do we do rolling restart?
4 participants