start.sh appears not to support integer arguments about marketplace-k8s-app-tools HOT 11 CLOSED

@moxious @whyrg Please update your vendor and see if this fixes things for you. Per my last comment, there is still an issue that has to be worked around if you need to specify APP_PARAMETERS to start.sh.

from marketplace-k8s-app-tools.

I see. As mentioned in #74, this seems to be solved.

I believe it is safe to say we will only support strings for app parameters.

from marketplace-k8s-app-tools.

@trironkk FYI

from marketplace-k8s-app-tools.

@vcanaa FYI

from marketplace-k8s-app-tools.

What the partner expected here was that the value of the properties would replace the corresponding field (defined by that same path core.numberOfServers) in values.yaml.

from marketplace-k8s-app-tools.

This seems to be a continuation of #64 as well.

We have an int example in the onboarding docs:

...
serverReplicas:
    title:  Server replicas
    type:  int
    default: 1
    minimum: 1

The serverReplicas param is a very common setting regardless of application. It's much easier for platform adoption is the integrator doesn't have to jump through hoops to get basic parameters working.

I understand there's a limit due to envsubst from the key comment by @huyhg in #74:

The reason for this restriction is because properties are also being passed as environment variables, particularly in the case where envsubst is used for template expansion instead of helm's go template.

Which manifests as the following:

Container | Timestamp | Message
-- | -- | --
deployer | May 8, 2018, 3:47:41 PM | TypeError: execve() arg 3 contains a non-string value
deployer | May 8, 2018, 3:47:41 PM | raise child_exception
deployer | May 8, 2018, 3:47:41 PM | File "/usr/lib/python2.7/subprocess.py", line 1024, in _execute_child
deployer | May 8, 2018, 3:47:41 PM | errread, errwrite)
deployer | May 8, 2018, 3:47:41 PM | File "/usr/lib/python2.7/subprocess.py", line 390, in __init__
deployer | May 8, 2018, 3:47:41 PM | stderr=subprocess.PIPE)
deployer | May 8, 2018, 3:47:41 PM | File "/bin/config_env.py", line 69, in main
deployer | May 8, 2018, 3:47:41 PM | main()
deployer | May 8, 2018, 3:47:41 PM | File "/bin/config_env.py", line 81, in <module>
deployer | May 8, 2018, 3:47:41 PM | Traceback (most recent call last):
deployer | May 8, 2018, 3:47:41 PM | + cat /data/manifest/aerospike.yaml.template
deployer | May 8, 2018, 3:47:41 PM | + /bin/config_env.py envsubst '<A whole bunch of $PARAMS>'
...

At the end of the day, we're populating a YAML file. Why not a jinja template instead?

from marketplace-k8s-app-tools.

Yes, the intention is to support parameters of types other than strings. The limitation due to envsubst is on the key/property name, not the value itself.

What @whyrg describes is a supported use case. Thanks for reporting the error--this is indeed a bug in config_env when dealing with non-string property types. I'll send a fix.

from marketplace-k8s-app-tools.

I've sent #80 which should fix this.

There is still an issue with start.sh when the user wants to pass parameter values on the command line, in which case the values have to be passed as strings. As an example, for the serverReplicas above, if you want to pass the json to start.sh (APP_PARAMETERS env var in make), it should be:

{ "serverReplicas": "3" }

instead of the expected:

{ "serverReplicas": 3 }

This issue is tracked in #63.

from marketplace-k8s-app-tools.

Confirmed this worked

from marketplace-k8s-app-tools.

Confirmed working as well

from marketplace-k8s-app-tools.

Confirmed working as well

from marketplace-k8s-app-tools.