search

bookingcom / shipper

Kubernetes native multi-cluster canary or blue-green rollouts using Helm
Apache License 2.0
736 stars 41 forks source link

Large integer `image tag` gets converted to scientific notation #102

Open Kiougar opened 5 years ago

Kiougar commented 5 years ago

When an image tag is a large integer (e.g. 20190612073634) it gets converted to the scientific notation (e.g 2.0190612073634e+13)

For example the following release configuration:

Values:
  Image:
    Repository:  [...]
    Tag:         20190612073634

Results in this pod configuration:

Containers:
  app:
    Container ID:
    Image:          [...]:2.0190612073634e+13

As a result, the pod is at InvalidImageName error state

karuppiah7890 commented 5 years ago

@Kiougar You can enclose the value in quotes or double quotes to stop this.

Tag: '20190612073634'

Kiougar commented 5 years ago

@karuppiah7890 thanks for the suggestion! What I'm currently doing is this instead:

Tag: v20190612073634

However, I still think the original issue is a bug and needs to be fixed.

jan25 commented 5 years ago

This is a problem originating deep down in dependency chain inside core yaml library: https://github.com/ghodss/yaml. Refer to this comment in helm based on their investigation.

osdrv commented 5 years ago

We made some little progress on this problem and opened a corresponding PR: https://github.com/helm/helm/pull/6010. If this change will appear in helm branch 2, we will be able to bump Shipper helm dependency version and fix the problem around version 0.6–0.7. Will keep this thread posted.

osdrv commented 4 years ago

A quick update on the issue: the fix is not coming any time soon.

The change caused a major regression in numeric functions (see https://github.com/helm/helm/issues/6708 for more details). Our attempt to close the breach turned into an unreasonably massive and ugly change https://github.com/helm/helm/pull/6709, which didn't look like a small fix at all. Taking into consideration potential risks, the fixing PR has been closed with no merge and the initial change reverted. There is a possibility the problem would be fixed in the future versions of Helm3 (the idea is to use reflect-based function overloads; we've made some initial progress here: https://github.com/icanhazbroccoli/helm@8b359af, but it's not close to anything working yet). My apologies for being over-optimistic and wasting so much time on it.

osdrv commented 4 years ago

A new hope to get this fixed comes with this PR: https://github.com/helm/helm/pull/6888. I also created another issue https://github.com/bookingcom/shipper/issues/228 as if the PR gets merged into Helm upstream, we will have to upgrade straight to helm 3. No ETA on getting this fixed yet. Keeping it open for now.

harrysingh commented 3 years ago

@osdrv What is latest state of the fix around this?

osdrv commented 3 years ago

Hey @harrysingh, after a few attempts to fix the issue the fix was not accepted by Helm folks: in the end it looked reflect-monstrous, I admit. Neither me nor the team can make any promises about the fix ever landing in Helm. In the meantime I'd suggest following the workarounds proposed in the corresponding Helm issue ticket.