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

2.0 prep: Remove deprecated flags #173

Merged
merged 3 commits into from
Aug 16, 2019
Merged

2.0 prep: Remove deprecated flags #173

merged 3 commits into from
Aug 16, 2019

Conversation

jsafrane
Copy link
Contributor

@jsafrane jsafrane commented Aug 15, 2019 •
edited
Loading

What type of PR is this?
/kind cleanup

What this PR does / why we need it:
In 2.0 we can safely remove deprecated flags.

Which issue(s) this PR fixes:
Related to #158

Does this PR introduce a user-facing change?:

Action required: the `-connection-timeout`, `-leader-election-type` and `-leader-election-identity` flags, deprecated in v1.2, have been removed. Please update your manifests for the external-attacher. Leader election uses `lease` object now. Rolling update from v1.2.y release may not work, as multiple leaders may be elected during the update (one using config maps and another using `lease` object).
Action required: the `-dummy` flag has been removed. Please update your manifests for the external-attacher.

@jsafrane
2.0 prep: Remove deprecated flags
c2ed912 
In 2.0 we can safely remove deprecated flags.
@k8s-ci-robot k8s-ci-robot added release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. 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 Aug 15, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jsafrane

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 approved Indicates a PR has been approved by an approver from all required OWNERS files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 15, 2019
@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 Aug 15, 2019
msau42
msau42 reviewed Aug 15, 2019
View reviewed changes
cmd/csi-attacher/main.go Outdated
kubeconfig = flag.String("kubeconfig", "", "Absolute path to the kubeconfig file. Required only when running out of cluster.")
resync = flag.Duration("resync", 10*time.Minute, "Resync interval of the controller.")
csiAddress = flag.String("csi-address", "/run/csi/socket", "Address of the CSI driver socket.")
dummy = flag.Bool("dummy", false, "Run in dummy mode, i.e. not connecting to CSI driver and marking everything as attached. Expected CSI driver name is \"csi/dummy\".")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Was this only for initial development? Can we remove this too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed -dummy too, I hope release note generator can handle multi-line comments :-)

@jsafrane
Remove --dummy parameter
298f850 
It is not used in any test now.
@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 16, 2019
@jsafrane
Copy link
Contributor Author

I removed -dummy. And when at it, I noticed that there was an old unused variable with the old CRD-based informers, so I deleted it (removing lot of files from vendor/)

@jsafrane jsafrane mentioned this pull request Aug 16, 2019
Closed
@msau42
Copy link
Collaborator

msau42 commented Aug 16, 2019

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 16, 2019
@k8s-ci-robot k8s-ci-robot merged commit c745c0f into kubernetes-csi:master Aug 16, 2019
@Colstuwjx Colstuwjx mentioned this pull request Mar 30, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@msau42 msau42 msau42 left review comments

@davidz627 davidz627 Awaiting requested review from davidz627

@sbezverk sbezverk Awaiting requested review from sbezverk

Assignees

@msau42 msau42

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. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jsafrane @k8s-ci-robot @msau42