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

Release Helm chart v3.8.3 #1146

Conversation

stevehipwell
Copy link
Contributor

@stevehipwell stevehipwell commented Dec 5, 2022

What this PR does / why we need it:
This PR releases the v3.8.3 Helm chart containing Metrics Server v0.6.2. I've had to leave the default container port and registry changes out of this release so they will be in the chart for v0.7.0 when that is released.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #1085
Fixes #1125
Fixes #1135

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Dec 5, 2022
@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 5, 2022
@stevehipwell
Copy link
Contributor Author

/triage accepted

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Dec 5, 2022
@stevehipwell
Copy link
Contributor Author

/retest

1 similar comment
@serathius
Copy link
Contributor

/retest

@yangjunmyfm192085
Copy link
Contributor

@stevehipwell , I think we need to check the helm chart. During the e2e test, there will be two deployments of metrics-server at the same time, such as
image

Because the deployment under the default namespace has not been ready, the test failed

@stevehipwell
Copy link
Contributor Author

@yangjunmyfm192085 how come there are two installations?

@yangjunmyfm192085
Copy link
Contributor

yangjunmyfm192085 commented Dec 5, 2022

I'm not sure. The guess may be that the helm chart specifies the namespace, and I am not sure which side of the value has an error

@stevehipwell
Copy link
Contributor Author

@yangjunmyfm192085 it looks like you fixed scaffold.yaml after the release branch was cut (#870). I'll port your changes for the Helm chart through which looks like they will fix this.

@stevehipwell
Copy link
Contributor Author

/retest

@stevehipwell
Copy link
Contributor Author

@yangjunmyfm192085 do you know why there is already an installation of metrics-server in the Kind cluster as part of the e2e tests?

@stevehipwell
Copy link
Contributor Author

@serathius the GH actions based Helm chart deployment is working correctly and the pull-metrics-server-test-e2e-helm check is working correctly on the master branch. I think it's going to be painful to get this check passing on this branch and I'm not sure what it would prove?

I've opened #1147 to update the automation which I've been looking into ready for the next release and will finish it once this release is completed so I can make any changes which might be required.

@yangjunmyfm192085
Copy link
Contributor

@yangjunmyfm192085 do you know why there is already an installation of metrics-server in the Kind cluster as part of the e2e tests?

pull-metrics-server-test-e2e-helm e2e test is to verify the deployment test of helm chart. After starting kind, the helm chart will be used to deploy the metrics-server, and then the e2e test will be performed.
Could you help analyze this error?

 - The Deployment "metrics-server" is invalid: spec.selector: Invalid value: v1.LabelSelector{MatchLabels:map[string]string{"app.kubernetes.io/instance":"metrics-server", "app.kubernetes.io/name":"metrics-server", "k8s-app":"metrics-server"}, MatchExpressions:[]v1.LabelSelectorRequirement(nil)}: field is immutable
kubectl apply: exit status 1
make[1]: *** [Makefile:151: test-e2e-1.24] Error 1
make[1]: Leaving directory '/home/prow/go/src/sigs.k8s.io/metrics-server'

skaffold.yaml Outdated Show resolved Hide resolved
Signed-off-by: Steve Hipwell <steve.hipwell@gmail.com>
@stevehipwell
Copy link
Contributor Author

/retest

1 similar comment
@stevehipwell
Copy link
Contributor Author

/retest

@stevehipwell
Copy link
Contributor Author

@yangjunmyfm192085 @serathius could you please review this now as the tests are passing correctly.

Copy link
Contributor

@yangjunmyfm192085 yangjunmyfm192085 left a comment

Choose a reason for hiding this comment

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

It seems ok to me. I think we need @serathius 's final review
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 6, 2022
@stevehipwell
Copy link
Contributor Author

@serathius @dgrisonnet could one of you take a look at this?

@dgrisonnet
Copy link
Member

/lgtm

@stevehipwell
Copy link
Contributor Author

@dgrisonnet do we need to wait for @serathius to approve or could you approve?

@dgrisonnet
Copy link
Member

I am not an approver so I can't approve. That said I have the permissions to merge but I'd rather not use them and follow the usual workflow. Let's wait a bit for Marek to have a look

@serathius
Copy link
Contributor

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: serathius, stevehipwell

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 Dec 8, 2022
@k8s-ci-robot k8s-ci-robot merged commit eb650ca into kubernetes-sigs:release-0.6 Dec 8, 2022
@stevehipwell stevehipwell deleted the release-helm-chart-v3-8-3 branch December 8, 2022 13:58
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. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet