kops Fix openstack tag limitation

Openstack is not allowing object tag size of more than 60 characters, as we can not rename a cluster we have to truncate and limit length to 42 for the tag value.

Jun 22 '22 13:06 akkina2107

The committers listed above are authorized under a signed CLA.

:white_check_mark: login: akkina2107 (27e7d4f0a44e1408c882c9a39010e2a977314e7f)

Jun 22 '22 13:06 linux-foundation-easycla[bot]

Welcome @akkina2107!

It looks like this is your first PR to kubernetes/kops 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/kops has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. :smiley:

Jun 22 '22 13:06 k8s-ci-robot

Hi @akkina2107. Thanks for your PR.

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

Jun 22 '22 13:06 k8s-ci-robot

Kops' GCE code has logic to truncate a cluster name such that it still contains a hash suffix, which would avoid similarly-named clusters from being truncated down to identical names. We could probably reuse that here

https://github.com/kubernetes/kops/blob/master/upup/pkg/fi/cloudup/gce/utils.go#L52-L71

Jun 22 '22 22:06 rifelpet

I used the proposed function, please verify

Jun 23 '22 09:06 akkina2107

we faced this issue while updating our clusters to kops 1.22 and we need the solution to be working for the same.

Jun 24 '22 09:06 akkina2107

This PR is very much related to this one: https://github.com/kubernetes/kops/pull/13861 Linking it here for reference.

Jun 24 '22 12:06 mitch000001

/easycla

Jun 24 '22 15:06 jarias-lfx

please update the current status of this PR

Jun 27 '22 07:06 akkina2107

/ok-to-test

Jul 04 '22 11:07 zetaab

@akkina2107 could you fix the tests. Otherwise it looks correct

Jul 04 '22 11:07 zetaab

/retest

Jul 05 '22 07:07 akkina2107

I would appreciate if someone can let me know, on which version this issue will be fixed? we need a solution for 1.22 version is the current solution back ported? if not how can I achieve this? kindly let me know.

Jul 06 '22 08:07 akkina2107

@akkina2107 could you fix the tests. Otherwise it looks correct

@zetaab wouldn't it be better instead of using the "magic" number 42 here to do it like @drekle suggested:

This would be a magic integer IMO. I think what you mean is something like MAX_TAG_LENGTH_OPENSTACK - len(openstack.TagClusterName) + 1 +1 for the = here, which just happens to be 42 however would impact anyone much more than necessary if they changed the value of the constant openstack.TagClusterName.

I believe this could be something more like:

const MAX_TAG_LENGTH_OPENSTACK = 60
func formatTag(tag string) string {
   if len(tag) <= MAX_TAG_LENGTH_OPENSTACK {
       return tag
   }
   return tag[:MAX_TAG_LENGTH_OPENSTACK ]
}
  	Tags: []string{
  		formatTag(fmt.Sprintf("%s=%s", openstack.TagKopsInstanceGroup, groupName)),
  		formatTag(fmt.Sprintf("%s=%s", openstack.TagKopsName, portTagKopsName)),
  		formatTag(fmt.Sprintf("%s=%s", openstack.TagClusterName, b.ClusterName())),

Sources:

https://github.com/kubernetes/kops/pull/13853#discussion_r904102119
https://github.com/kubernetes/kops/pull/13853#discussion_r904105869

And also, like @mitch000001 pointed out, my changes in #13861 need to be adapted as well (https://github.com/kubernetes/kops/pull/13861/files#diff-948b4589cbdb93053f2dbe3ee8a1e710d3d171030bcfe2b4efdbba88ad8c80eeR122)

Jul 07 '22 14:07 ederst

/retest

Jul 19 '22 12:07 akkina2107

/retest

Jul 19 '22 12:07 akkina2107

@zetaab and @ederst @drekle Please inspect the new changes and leave your comments

Jul 19 '22 12:07 akkina2107

@zetaab Please take a look this PR. Can it be merged ?

Aug 12 '22 08:08 DavidSie

@zetaab please verify

Aug 30 '22 08:08 akkina2107

/test pull-kops-e2e-aws-karpenter

Aug 31 '22 08:08 akkina2107

/retest

Aug 31 '22 08:08 akkina2107

@zetaab kindly review

Sep 05 '22 07:09 akkina2107

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: zetaab

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:

~~pkg/model/openstackmodel/OWNERS~~ [zetaab]
~~upup/pkg/fi/cloudup/openstacktasks/OWNERS~~ [zetaab]

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

Sep 06 '22 18:09 k8s-ci-robot

Probably want to cherry pick this one to release-1.25

Sep 06 '22 18:09 olemarkus