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.

Sign up for GitHub

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

Jump to bottom

updated pinning to prevent breaking changes #4783

Merged
merged 5 commits into from
May 20, 2019
Merged

updated pinning to prevent breaking changes #4783

merged 5 commits into from
May 20, 2019

Conversation

marcstreeter
Copy link
Contributor

What type of PR is this?
kind bug

What this PR does / why we need it:
Currently the pinning scheme allows for more breaking changes than is necessary. >=2.7.8 could give you 2.8.0 or 2.7.10 depending on when you ran it. Ansible 2.8.0 breaks deploying Kubespray when 2.7.10 does not. This PR would still allow for the 2.7.8 or newer versions of Ansible while not jumping to 2.8.*.
Which issue(s) this PR fixes:

Fixes #3985

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

action required, must re-run `pip install -r requirements`

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label May 18, 2019
@k8s-ci-robot k8s-ci-robot requested review from Miouge1 and mirwan May 18, 2019 14:35
@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label May 18, 2019
@marcstreeter
Copy link
Contributor Author

/assign @ant31

@Miouge1
Copy link
Contributor

Miouge1 commented May 18, 2019

ci check this

@woopstar
Copy link
Member

This is a good idea. But can we pin all requirements to a specific version. We should pin Ansible to 2.7 as here, and the rest to the last version gathered with pip install.

This would prevent breakage in the future.

@woopstar
Copy link
Member

/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 19, 2019
@woopstar woopstar mentioned this pull request May 19, 2019
Closed
@marcstreeter
Copy link
Contributor Author

totally agreed. I prefer strict pinning. I could update this so that each one is a == instead.

@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels May 20, 2019
@marcstreeter
Copy link
Contributor Author

pinned all to a specific version and left it at Ansible 2.7.8 as requested.

@woopstar
Copy link
Member

Great ! There is a requirements file in the tests folder too. Can you upgrade that one too.

@marcstreeter
Copy link
Contributor Author

added testing pinning information. One note, boto==2.9.0 seems like it's the last python 2 specific requirement we have. Probably would be good to move off of that before python 2 is EOL'ed.

@woopstar
Copy link
Member

I dont think boto is used anymore. You can try to remove it.

@woopstar
Copy link
Member

/lgtm
/approve

Looks good. This should make things a bit more stable.

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: marcstreeter, woopstar

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 added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 20, 2019
@woopstar
Copy link
Member

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 20, 2019
@k8s-ci-robot k8s-ci-robot merged commit 872b37f into kubernetes-sigs:master May 20, 2019
@marcstreeter marcstreeter deleted the less-permissive-pinning branch May 21, 2019 01:08
unbreakab1e pushed a commit to joomcode/kubespray that referenced this pull request Jun 5, 2019
@marcstreeter @unbreakab1e
updated pinning to prevent breaking changes (kubernetes-sigs#4783)
507a622 
* updated ansible pinning to prevent more possibilities of breaking changes

* more exact pinning of ansible version

* more exact pinning of ansible version and also all the rest

* added testing requirements.txt pinning settings

* removed boto from testing requirements.txt
holmsten pushed a commit to holmsten/kubespray that referenced this pull request Jun 13, 2019
@marcstreeter @holmsten
updated pinning to prevent breaking changes (kubernetes-sigs#4783)
6b0636e 
* updated ansible pinning to prevent more possibilities of breaking changes

* more exact pinning of ansible version

* more exact pinning of ansible version and also all the rest

* added testing requirements.txt pinning settings

* removed boto from testing requirements.txt
k8s-ci-robot pushed a commit that referenced this pull request Jun 13, 2019
@holmsten @k8s-ci-robot
updated pinning to prevent breaking changes (#4783) (#4873)
b90b1fc 
* updated ansible pinning to prevent more possibilities of breaking changes

* more exact pinning of ansible version

* more exact pinning of ansible version and also all the rest

* added testing requirements.txt pinning settings

* removed boto from testing requirements.txt
LuckySB pushed a commit to southbridgeio/kubespray that referenced this pull request Aug 4, 2019
@marcstreeter @LuckySB
updated pinning to prevent breaking changes (kubernetes-sigs#4783)
acee3a0 
* updated ansible pinning to prevent more possibilities of breaking changes

* more exact pinning of ansible version

* more exact pinning of ansible version and also all the rest

* added testing requirements.txt pinning settings

* removed boto from testing requirements.txt
@vielmetti vielmetti mentioned this pull request Aug 7, 2019
Closed
@holmsten holmsten mentioned this pull request Aug 22, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Miouge1 Miouge1 Awaiting requested review from Miouge1

@mirwan mirwan Awaiting requested review from mirwan

Assignees

@ant31 ant31

@woopstar woopstar

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. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

'delegate_to' is not a valid attribute for a TaskInclude
5 participants
@marcstreeter @Miouge1 @woopstar @k8s-ci-robot @ant31