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

Add changelog for v2.2.0 #605

Merged
merged 1 commit into from
Apr 14, 2021
Merged

Add changelog for v2.2.0 #605

merged 1 commit into from
Apr 14, 2021

Conversation

chrishenzie
Copy link
Contributor

What type of PR is this?
/kind documentation

What this PR does / why we need it:
This PR adds the changelog for v2.2.0.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

/assign @msau42

@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. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Apr 9, 2021
@k8s-ci-robot k8s-ci-robot requested review from davidz627 and lpabon April 9, 2021 23:12
@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 9, 2021
@chrishenzie
Copy link
Contributor Author

/assign @pohly

@chrishenzie
Copy link
Contributor Author

/hold

The README needs updating.

@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 Apr 9, 2021
@chrishenzie
Copy link
Contributor Author

/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 Apr 9, 2021
chrishenzie
chrishenzie commented Apr 9, 2021
View reviewed changes
README.md Outdated
@@ -13,7 +13,7 @@ This information reflects the head of this branch.

| Compatible with CSI Version | Container Image | [Min K8s Version](https://kubernetes-csi.github.io/docs/kubernetes-compatibility.html#minimum-version) | [Recommended K8s Version](https://kubernetes-csi.github.io/docs/kubernetes-compatibility.html#recommended-version) |
| ------------------------------------------------------------------------------------------ | -------------------------------| --------------- | ------------- |
| [CSI Spec v1.0.0](https://github.com/container-storage-interface/spec/releases/tag/v1.0.0) | k8s.gcr.io/sig-storage/csi-provisioner | 1.17 | 1.19 |
| [CSI Spec v1.0.0](https://github.com/container-storage-interface/spec/releases/tag/v1.0.0) | k8s.gcr.io/sig-storage/csi-provisioner | 1.17 | 1.21 |
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I update this to v1.4.0 or make this a range of versions 1.0.0-1.4.0?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's update it to 1.4.0. The previous version compatibility is implied.

msau42
msau42 reviewed Apr 10, 2021
View reviewed changes
Copy link
Collaborator

@msau42 msau42 left a comment

Choose a reason for hiding this comment

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

Can you also do a comparison to make sure we have release-notes for important feature and bug fix prs?

CHANGELOG/CHANGELOG-2.2.md Outdated

- After volume creation or deletion, those CSIStorageCapacity objects most likely affected by that get refreshed sooner than the others. ([#586](https://github.com/kubernetes-csi/external-provisioner/pull/586), [@pohly](https://github.com/pohly))
- New metrics data (storage capacity tracking, process and Go runtime, work queues, leaderelection) ([#579](https://github.com/kubernetes-csi/external-provisioner/pull/579), [@pohly](https://github.com/pohly))
- Set the value of "migrated" field in the metrics to true or false to indicate if the call is a migration enabled feature or not ([#560](https://github.com/kubernetes-csi/external-provisioner/pull/560), [@nearora-msft](https://github.com/nearora-msft))
Copy link
Collaborator

Choose a reason for hiding this comment

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

I updated the release notes wording on a number of bugs. Can you regenerate?

CHANGELOG/CHANGELOG-2.2.md
### Feature

- After volume creation or deletion, those CSIStorageCapacity objects most likely affected by that get refreshed sooner than the others. ([#586](https://github.com/kubernetes-csi/external-provisioner/pull/586), [@pohly](https://github.com/pohly))
- New metrics data (storage capacity tracking, process and Go runtime, work queues, leaderelection) ([#579](https://github.com/kubernetes-csi/external-provisioner/pull/579), [@pohly](https://github.com/pohly))
Copy link
Collaborator

Choose a reason for hiding this comment

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

@pohly can we add a doc referenced from the README that lists all the metrics that we export? Or maybe a command that someone can run against the metrics endpoint to get the help text for all the metrics?

Copy link
Contributor

@pohly pohly Apr 10, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

There is kubernetes-csi/docs#339 open for metrics documentation. This is a bigger issue because there's basically no documentation at all at the moment and I wasn't sure where to start when adding the new ones.

@chrishenzie
Copy link
Contributor Author

Can you also do a comparison to make sure we have release-notes for important feature and bug fix prs?

Done, all of them have release-notes.

@chrishenzie
Copy link
Contributor Author

chrishenzie commented Apr 10, 2021 •
edited
Loading

Is this test failure a flake? When I ran make test locally the capacity suite took 320 seconds to run. Is this expected?
Edit: I guess so? It took prow 309 seconds to run this.

@msau42
Copy link
Collaborator

msau42 commented Apr 10, 2021

Is this test failure a flake? When I ran make test locally the capacity suite took 320 seconds to run. Is this expected?
Edit: I guess so? It took prow 309 seconds to run this.

@pohly any way we can parallelize the capacity tests so it runs faster?

@chrishenzie chrishenzie force-pushed the release-2.2 branch 2 times, most recently from a75b23f to 10634b9 Compare April 10, 2021 05:45
@pohly
Copy link
Contributor

pohly commented Apr 10, 2021

any way we can parallelize the capacity tests so it runs faster?

The tests use metrics that are global instances and therefore the tests cannot run in parallel. Log output also goes to the global klog instance, but that at least doesn't affect test execution, only test analysis after a failure.

I'll check whether it is possible to avoid the global metrics instances when testing.

@pohly
Copy link
Contributor

pohly commented Apr 10, 2021

any way we can parallelize the capacity tests so it runs faster?

The tests use metrics that are global instances and therefore the tests cannot run in parallel. Log output also goes to the global klog instance, but that at least doesn't affect test execution, only test analysis after a failure.

I'll check whether it is possible to avoid the global metrics instances when testing.

Duh! I had already changed that and then promptly forgot about it. Therefore parallelizing the tests was pretty straight-forward: #607

@chrishenzie
Copy link
Contributor Author

/hold

Until we get #607 merged. Would prefer for the changelog to be the final commit prior to release. Still waiting on being added to the maintainers.

@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 Apr 12, 2021
@chrishenzie
Copy link
Contributor Author

/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 Apr 12, 2021
msau42
msau42 reviewed Apr 12, 2021
View reviewed changes
CHANGELOG/CHANGELOG-2.2.md Outdated

### Other (Cleanup or Flake)

- Removed redundant log lines for CreateVolume and GetCapacity requests. ([#604](https://github.com/kubernetes-csi/external-provisioner/pull/604), [@chrishenzie](https://github.com/chrishenzie))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you also update this to match the wording of the one in 2.1?

Copy link
Contributor Author

@chrishenzie chrishenzie Apr 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Done (updated on PR, re-ran script to generate this)

@chrishenzie
Copy link
Contributor Author

chrishenzie commented Apr 12, 2021 •
edited
Loading

@pohly When you get a moment can you take a look at the above test failure? Here is another failure.

@chrishenzie
Copy link
Contributor Author

/retest

@msau42
Copy link
Collaborator

msau42 commented Apr 12, 2021

/lgtm
/approve

Not sure if you need to sync up to get #607

@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 Apr 12, 2021
@chrishenzie
Copy link
Contributor Author

This is rebased on master and has the changes to parallel execute tests

@chrishenzie
Copy link
Contributor Author

/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 Apr 13, 2021
@pohly
Copy link
Contributor

pohly commented Apr 13, 2021

When you get a moment can you take a look at the above test failure? Here is another failure.

Sorry, I missed the ping and only saw it now. I'll take a look tomorrow.

A dedicated issue for this is better, I created one: #609

@chrishenzie
Add changelog for v2.2.0
d20d93b 
And update compatibility and feature status tables in README.
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 14, 2021
@chrishenzie
Copy link
Contributor Author

/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 Apr 14, 2021
@msau42
Copy link
Collaborator

msau42 commented Apr 14, 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 Apr 14, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chrishenzie, msau42

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 213cd3d into kubernetes-csi:master Apr 14, 2021
@chrishenzie chrishenzie deleted the release-2.2 branch April 14, 2021 21:09
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

@pohly pohly pohly left review comments

@davidz627 davidz627 Awaiting requested review from davidz627

@lpabon lpabon Awaiting requested review from lpabon

Assignees

@pohly pohly

@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/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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@chrishenzie @msau42 @pohly @k8s-ci-robot