Skip to content
This repository has been archived by the owner on Feb 22, 2022. It is now read-only.

[stable/superset] Use the target port number instead of a name #7857

Merged
merged 2 commits into from
Feb 15, 2019
Merged

[stable/superset] Use the target port number instead of a name #7857

merged 2 commits into from
Feb 15, 2019

Conversation

legal90
Copy link
Contributor

@legal90 legal90 commented Sep 20, 2018

Older (but still actual) versions of some Network Policy providers don't support named ports as
a value of targetPort field in the Service spec:

Changing it to the number shouldn't break the UX for existing installations because the container port 8088 is still hard-coded here:

charts/stable/superset/templates/deployment.yaml

Line 57 in 6b518ce

containerPort: 8088

@ey-bot ey-bot added the Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). label Sep 20, 2018
@k8s-ci-robot
Copy link
Contributor

Hi @legal90. Thanks for your PR.

I'm waiting for a helm member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

I understand the commands that are listed here.

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 added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Sep 20, 2018
@stale
Copy link

stale bot commented Oct 20, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Any further update will cause the issue/pull request to no longer be considered stale. Thank you for your contributions.

@stale stale bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 20, 2018
@legal90
Copy link
Contributor Author

legal90 commented Oct 20, 2018

It's not stale

@stale stale bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 20, 2018
@stale
Copy link

stale bot commented Nov 19, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Any further update will cause the issue/pull request to no longer be considered stale. Thank you for your contributions.

@stale stale bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 19, 2018
@legal90 legal90 force-pushed the superset-target-port branch from da1ff96 to 8207b6e Compare November 19, 2018 12:38
@helm-bot helm-bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Nov 19, 2018
@stale stale bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 19, 2018
@legal90
Copy link
Contributor Author

legal90 commented Nov 19, 2018

@linki @foxish Could you please review this PR? Cheers!

@legal90 legal90 force-pushed the superset-target-port branch from 8207b6e to 3b8bd47 Compare November 19, 2018 12:40
@helm-bot helm-bot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Nov 19, 2018
@legal90 legal90 force-pushed the superset-target-port branch from 3b8bd47 to 923a161 Compare November 22, 2018 11:57
@helm-bot helm-bot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Nov 22, 2018
@stale
Copy link

stale bot commented Dec 22, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Any further update will cause the issue/pull request to no longer be considered stale. Thank you for your contributions.

@stale stale bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 22, 2018
@legal90 legal90 force-pushed the superset-target-port branch from 923a161 to 36d1170 Compare December 22, 2018 20:15
@helm-bot helm-bot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Dec 22, 2018
@stale stale bot removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Dec 22, 2018
@legal90
Copy link
Contributor Author

legal90 commented Jan 7, 2019

/assign @prydonius

@legal90
[stable/superset] Use the target port number instead of a name
ce230f5 
Older (but still actual) versions of some Network Policy providers don't support named ports as
a value of `targetPort` field in the Service spec.
Changing it to the number shouldn't break the UX for existing installations.

Signed-off-by: Mikhail Zholobov <[email protected]>
@legal90
[stable/superset] Bump version
98d74e2 
Signed-off-by: Mikhail Zholobov <[email protected]>
@legal90 legal90 force-pushed the superset-target-port branch from 36d1170 to 98d74e2 Compare January 31, 2019 14:52
@helm-bot helm-bot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jan 31, 2019
@legal90
Copy link
Contributor Author

legal90 commented Jan 31, 2019

@prydonius @cpanato Could you please review?

@cpanato
Copy link
Member

cpanato commented Feb 15, 2019

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Feb 15, 2019
@cpanato
Copy link
Member

cpanato commented Feb 15, 2019

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 15, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cpanato, legal90

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 Feb 15, 2019
@k8s-ci-robot k8s-ci-robot merged commit 0facbbc into helm:master Feb 15, 2019
@legal90 legal90 deleted the superset-target-port branch February 15, 2019 13:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@foxish foxish Awaiting requested review from foxish

@linki linki Awaiting requested review from linki

Assignees

@prydonius prydonius

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. Contribution Allowed If the contributor has signed the DCO or the CNCF CLA (prior to the move to a DCO). lgtm Indicates that a PR is ready to be merged. ok-to-test size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@legal90 @k8s-ci-robot @cpanato @prydonius @ey-bot @helm-bot