Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

config: No text/template wrapper #491

Merged
merged 6 commits into from
Jul 7, 2021
Merged

config: No text/template wrapper #491

merged 6 commits into from
Jul 7, 2021

Conversation

kellerza
Copy link
Contributor

@kellerza kellerza commented Jul 5, 2021

Nr 1 from #487

Implementation similar to the existing logic from https://github.com/kellerza/template/blob/main/template.go#L59

@karimra mentioned there is a built in method to search the paths, but did not find it.

@karimra
Copy link
Member

karimra commented Jul 6, 2021

you can do something like this:

func (c *CLab) parseTemplates(pathToTemplates string) (*template.Template, error) {
    t := template.New(c.Config.Name)
    err := filepath.Walk(pathToTemplates, func(path string, info os.FileInfo, err error) error {
        if strings.HasSuffix(path, ".gotmpl") { // or any other condition identifying the template files
            _, err = t.ParseFiles(path)
           return err
    })

    return t, err
}

The individual templates can be called with their name, which defaults to the file base name.

t.ExecuteTemplate(os.Stdout, name, vars)

you can get the list of templates associated with t:

t.Templates()

you can lookup a specific template associated with t:

t.Lookup(name)

clab/config/template.go Outdated Show resolved Hide resolved
@kellerza
Copy link
Contributor Author

kellerza commented Jul 6, 2021

Thanks @karimra. Changed to ExecuteTemplate

Not sure walking the root path is applicable. The idea here is to look for a specific file if you have a list of possible paths.

clab/config/template.go Outdated Show resolved Hide resolved
@karimra
Copy link
Member

karimra commented Jul 6, 2021

It's still possible to parse the list of directories, and call the desired template by name.
I'm not sure about the logic you want to implement.

Parsing the whole directory, helps combining templates from different files as well.

@kellerza
Copy link
Contributor Author

kellerza commented Jul 6, 2021

If we need templates from multiple files that is a requirement I did not have in mind. Do you see this as a requirement?

It seems quite ineffective, because of the possible amount of templates in a folder. You will end up loading all templates, for all kinds, over all folders specified.

If that is indeed the case, we should just run ParseGlob over all folders in order. From the docs:

When parsing multiple files with the same name in different directories, the last one mentioned will be the one that results.

Maybe with globs we could keep some of the existing logic, but only load all templates for the active role/kind

@karimra
Copy link
Member

karimra commented Jul 6, 2021

Templates can be combined by importing each other. In that case you would need to load the main template as well as the ones it imports.

Separating templates in multiples files allows reuse and separation of responsibilities/concerns, it's really at the core of the template engine design.

I don't think this has to be addressed in this PR, since we didn't really come to a conclusion concerning how templates will be mapped to kinds and nodes.

Can explain how a template file is mapped to a node in this PR? Is it a file name that gets inherited from kinds/defaults like the other variables?

@kellerza
Copy link
Contributor Author

kellerza commented Jul 6, 2021

You can pass a list of templates names. The template names will be combined with the role: <tname>__<role>.tmpl

By default the role is the kind, except if you specify it in vars.

@hellt
Copy link
Member

hellt commented Jul 6, 2021

You can pass a list of templates names. The template names will be combined with the role: <tname>__<role>.tmpl

By default the role is the kind, except if you specify it in vars.

yes, the idea here is that the template name is not related to a single node, but to a group of them. The default group marker is its kind.

So for a lab that has srl and sros kinds, this will allow to pass a template named interfaces and the files interfaces__srl.tmpl and interfaces__sros.tmpl will be looked up and applied to the relevant nodes

@kellerza
Copy link
Contributor Author

kellerza commented Jul 6, 2021

I would phrase the group of them slightly differently:

  • The template <name> gets applied to all nodes in the topology file.
  • The role/kind decides which template (SRL syntax,SROS syntax, etc) get rendered for that node.

Any finer control can be done inside the template with if constructs using even more of the vars on the node/link level.

@kellerza
Copy link
Contributor Author

kellerza commented Jul 6, 2021

The default group marker is its kind

Indeed the role variable creates groups:

  1. firstly each node kind will have its own CLI syntax (default=kind), but
  2. nodes can also be grouped into roles (like spine or leaf roles, or even leaf_srl and leaf_sros)

@kellerza kellerza closed this Jul 6, 2021
@kellerza kellerza reopened this Jul 6, 2021
@karimra
Copy link
Member

karimra commented Jul 6, 2021

it seems all this assumes that templates are being run with separate command, not run on deploy ?
can you show a topology file example where the templates are specified and the command to apply them ?

@kellerza
Copy link
Contributor Author

kellerza commented Jul 6, 2021

Indeed, config only deploys to running labs.
Vr-05 example folder has 2 examples and the base templates are in the active branch

try:

clab config -t topo.yml -n base -p ../templates/config

add -c 100 to view the rendered templates if you dont have an active lab

@karimra
Copy link
Member

karimra commented Jul 6, 2021

ok, if this is the case currently, ParseGlob works for now. I guess the glob can be adjusted to the pattern that matches the node name/role

@hellt
Copy link
Member

hellt commented Jul 7, 2021

@karimra do you have anything else you had in mind for this PR?
otherwise we can merge it

@karimra
Copy link
Member

karimra commented Jul 7, 2021

it's still hard for me to see the whole picture, go ahead. once the other PR is done it will hopefully be clearer.

@hellt hellt merged commit ef66122 into srl-labs:master Jul 7, 2021
@kellerza
Copy link
Contributor Author

kellerza commented Jul 7, 2021

I'll make some slides to capture the whole picture, I've been asked to present it to an internal community as well.

This should also give a start for the site/docs.

@kellerza kellerza deleted the unwrap branch July 7, 2021 09:19
@karimra
Copy link
Member

karimra commented Jul 7, 2021

I get how it works and how to use it, what I don't get is the Go code.
There seems to be a lot of assumptions on the template format and nature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants