api

api copied to clipboard

Added a short name to EnvoyFilter as well as other CRDs.

😊 Welcome @micnncim! This is either your first contribution to the Istio api repo, or it's been awhile since you've been here.

You can learn more about the Istio working groups, code of conduct, and contributing guidelines by referring to Contributing to Istio.

Thanks for contributing!

Courtesy of your friendly welcome wagon.

Hi @micnncim. Thanks for your PR.

I'm waiting for a istio 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.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

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.

/ok-to-test

Should we avoid taking over 2-letter names for alpha APIs ? I'm not sure how k8s reacts if different products register short names, and we may already have a problem with the k8s Gateways if they start registering short name.

At least the 'short name' should have a failsafe setting, I suspect install would fail if there is a conflict ( I'm more concerned of what will happen with gateways for this @howardjohn )

Not blocking this, just thinking out loud.

If there is a conflict k8s picks one of them to "win" when you type kubectl get ef. I am not sure how it picks, it seems kubectl get gateway always picks the k8s one. I opened an issue in k8s to make it give a warning on conflict.

On Tue, Nov 17, 2020 at 8:01 AM Costin Manolache [email protected] wrote:

Should we avoid taking over 2-letter names for alpha APIs ? I'm not sure how k8s reacts if different products register short names, and we may already have a problem with the k8s Gateways if they start registering short name.

At least the 'short name' should have a failsafe setting, I suspect install would fail if there is a conflict ( I'm more concerned of what will happen with gateways for this @howardjohn https://github.com/howardjohn )

Not blocking this, just thinking out loud.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/istio/api/pull/1756#issuecomment-729025967, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEYGXPG6VWZJD77YM7IN3LSQKM4BANCNFSM4TXOO7LQ .

Thank you for point out the issue.

I've checked the related issues and PRs (https://github.com/kubernetes/kubernetes/issues/94860, https://github.com/kubernetes-sigs/service-apis/pull/426).

The best solution would be that k8s takes care of this issue like warning, right? Because we can't expect a conflict with other CRDs completely.

However, since it's not certain k8s will take care of it, would it be better to take more than 2 letter word like evf, efl? IMO, typing something like kubectl get envoyfilter every time would be annoying for some of the users.

I wonder if we should have a policy against adding short names to alpha apis? Seems like it could be done as part of moving an api to beta. Thoughts?

/retest

@micnncim: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
release-notes_api	0142fcbeceda2505d7aff45d9cf74fe85ef08e77	link	/test release-notes_api

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. I understand the commands that are listed here.

/release-note-none

All our APIs are alpha now, there isn't any harm even conflict with other CRs, is there?

@micnncim: PR needs rebase.

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.

🚧 This issue or pull request has been closed due to not having had activity from an Istio team member since 2020-11-18. If you feel this issue or pull request deserves attention, please reopen the issue. Please see this wiki page for more information. Thank you for your contributions.

Created by the issue and PR lifecycle manager.