[Fuego] [PATCH] Allow ftc script to properly use Jenkins configured port

[Gustavo Sverzut Barbieri]

On Thu, Feb 8, 2018 at 6:33 PM, Bird, Timothy <Tim.Bird at sony.com> wrote:
>
>
>> -----Original Message-----
>> From: Gustavo Sverzut Barbieri
>> On Thu, Feb 8, 2018 at 5:17 PM,  <Tim.Bird at sony.com> wrote:
>> >> -----Original Message-----
>> >> From: Guilherme Campos Camargo
>> >>
>> >> Hello, Tim. Please see my modifications below. I was looking for an
>> >> utils directory/submodule with existing python stuff where I could store
>> >> the newly added read_conf function, but haven't found any. Given that
>> >> the function file is very straightforward, I just left it in here, right
>> >> next to your other utility function. Please let me know if you'd rather
>> >> have it moved somewhere else.
>> > Probably, but I'm not sure where yet
>> >
>> >>
>> >> Anyway, I think it would be nice if we could keep these and other python
>> >> utility funcions in a separate submodule in the future, so that they can
>> >> be easily imported by different python scripts. Please let me know what
>> >> you think about that.
>> > I agree completely.  I suspect other files may have the hardcoded 8080 in
>> > them, and sharing a utility routine to read the configuration will be good.
>> > Also, it's unknown what other features and data we'll add to conf file, so it
>> would
>> > be nice to have a single routine that reads it, in case it gets gnarly later.
>> >
>> > This is OK placement for now, though.
>> >>
>> >> Thanks
>> >>
>> >> --
>> >> Commit Message:
>> >>
>> >> With the possibility of configuring jenkins porth through
>> >> fuego/ro/conf/fuego.conf, a change on ftc script is needed so that it
>> >> uses the configured port and not the default one (8080).
>> >>
>> >> This patch solves that issue by using the ${jenkins_port} environment
>> >> variable (set on entrypoint.sh) instead of the hardcoded 8080.
>> >>
>> >> Signed-off-by: Guilherme Campos Camargo <guicc at profusion.mobi>
>> >> ---
>> >>  engine/scripts/ftc | 30 ++++++++++++++++++++++--------
>> >>  1 file changed, 22 insertions(+), 8 deletions(-)
>> >>
>> >> diff --git a/engine/scripts/ftc b/engine/scripts/ftc
>> >> index 06dadcf..2cf0019 100755
>> >> --- a/engine/scripts/ftc
>> >> +++ b/engine/scripts/ftc
>> >> @@ -60,19 +60,33 @@ import glob
>> >>  # MAJOR, MINOR, REVISION
>> >>  VERSION = (1,2,1)
>> >>
>> >> -# define these as globals
>> >> -log = None
>> >> -tail_fd = None
>> >> -
>> >>  # here's a utility routine to print a variable with it's name and value
>> >>  def pvar(name):
>> >>      caller = sys._getframe(1)
>> >>      print "DEBUG: python var %s=%s" % (name, caller.f_locals[name])
>> >>
>> >> +# here's a utility routine that reads from a configuration file where
>> configs
>> >> +# are stored as KEY=VALUE pairs (as .env files)
>> >> +def read_conf(fname):
>> >> +    def split_pair(line):
>> >> +        return line.strip().split('=', 1)
>> >> +    with open(fname) as f:
>> >> +        pairs = [split_pair(line) for line in f.readlines()]
>> >> +        return dict(pairs)
>> > I'll probably expand this a bit, to handle blank lines and lines preceded with
>> '#'.
>> > It might be good to get one more strip in there as well, on the split items
>> > to handle stuff like:
>> > '   foo   =      bar    '
>> > But I'll do that in a separate patch (and as a lower priority change).
>>
>> hi tim, so could you consider moving to Python's ConfigParser? Saves
>> code, uniform file format... just misses the "source conf" in bash,
> Yeah, that source conf in bash seems like a bit of a hack.  But I do like the
> simplicity of handling it in entrypoint.sh directly.
>
>> but I wonder if that will be needed. In any case you can provide a
>> tool such as others do (docker-machine env, etc), which would convert
>> to plain shell variables (basically read and print as "%s=%r" % (key,
>> value))
> That's possible.  How would entrypoint.sh read this without going through
> a byzantine process of read the config, save envs to a file, sourcing the
> file?
>
> I mean I could have done something like this, for just this simple line:
> (and ignored the format of the rest of the file)
> jenkins_port=$(grep jenkins_port /fuego-ro/conf/fuego.conf | sed "s/^jenkins_port=//")
>
> That has its own ugliness, when the shell supports reading this format directly.

well, the `ftc env` that produces "shell-evaluatable output"
(analogous to gpg-agent, docker-machine, etc...) is good, or simply
`ftc config get` as you said....

more reliable than trying to grep/sed, as things like escapes, quotes
would be handled properly.


>> > I love the simplicity of this routine, by the way.
>> >
>> >> +
>> >> +# define these as globals
>> >> +log = None
>> >> +tail_fd = None
>> >> +
>> >> +FUEGO_CFG_PATH = '/fuego-ro/conf/fuego.conf'
>> >> +fuego_config = read_conf(FUEGO_CFG_PATH)
>> > This needs to be done after the check for whether we're inside the
>> > docker container, or check the environment for FUEGO_RO.
>> >
>> > ftc run outside the container won't be able to read the configuration
>> > file from under /fuego-ro (that is, at the root).
>> >
>> > See the code for checking inside_docker() at the top of the 'main' function.
>> >
>> > If it's too complicated to move the read_conf, then for now, I'd be satisfied
>> > with checking for the existence of '/fuego-ro', and just returning an empty
>> dictionary.
>> > The only thing that an outside-the-container 'ftc' does is immediately call
>> 'ftc'
>> > inside the container.  So it shouldn't need any configuration values.
>> > (cross our fingers and hope it stays that way....)
>> > We don't really have a mechanism to figure out where the conf file is when
>> > 'ftc' is outside the container (especially if there are multiple fuegos running
>> > around), so we don't have much choice.
>>
>> something that I'm wondering: why do we ever need to run ftc *outside*
>> of the container? shouldn't a bash-alias solve this issue for people
>> that want to do that?
>
> No.  It's not a given that fuego will always be running in a container.
> Some people have complained about the overhead of the container.
> If you have the necessary stuff installed on your host, and don't want
> to use Jenkins as your front end, there actually only a few things
> required to allow you to run Fuego on the command line as a regular host
> process.  (e.g. you could do 'ftc run-test'  and not talk to a container).
>
> This is an incomplete feature now, that I wanted to allow for in the future.
> (Of course, figuring out where the FUEGO_RO, FUEGO_CORE and FUEGO_RW
> dirs. would be essential for this.  My plan was to put these into a section
> of the conf file, and thus reduce the required explicit information down to one
> path.)

ok, but in this case, AFAIU, there is no need to communicate or handle
specially.

running ftc inside the container should be the same as running ftc
outside of the container, given that outside of the container you
properly configured the environment. Basically "inside the container"
is just a way to get everything configured for you.

However running the ftc outside the container, expecting it to proxy
to inside the container, IMO, it's something we can avoid.

-- 
Gustavo Sverzut Barbieri
http://profusion.mobi embedded systems
--------------------------------------
MSN, GTalk, FaceTime: barbieri at gmail.com
Skype: gsbarbieri
Mobile: +55 (16) 99354-9890