libcontainer/intelrdt: IntelRdtManager refactoring

[xiaochenshen]

This is a set of patches for IntelRdtManager refactoring.
It is similar to part of the cgroup refactoring in #2387.

No functional change.
Please review commits one by one.

Signed-off-by: Xiaochen Shen [email protected]

[xiaochenshen]

@kolyshkin @AkihiroSuda @Creatone
Could you help review this PR at your convenience? Thank you!

[Creatone]

@xiaochenshen Can it wait till Monday? I'm on holidays now.

[xiaochenshen]

@xiaochenshen Can it wait till Monday? I'm on holidays now.

@Creatone No problem. Thank you for help! 😃

[Creatone]

LGTM

[kolyshkin]

LGTM

[Creatone]

@AkihiroSuda

[kolyshkin]

This broke cadvisor but the fix is trivial (google/cadvisor@48e7375#diff-c1ad83c3e8484f4f134e3fb0eff9666f7b19db23d0c20bd5476920ff1bcd9709)

By the way @xiaochenshen @Creatone why do we have to specify empty config, like this:

mgr := intelrdt.NewManager(&configs.Config{IntelRdt: &configs.IntelRdt{}}, id, resctrlPath)

Can we do something like

func NewManager(config *configs.Config, id string, path string) Manager {
        if config == nil {
                config = &configs.Config{IntelRdt: &configs.IntelRdt{}}
        }
        return &intelRdtManager{
                config: config,
                id:     id,
                path:   path,
        }
}

or it does not make any sense?

[xiaochenshen]

This broke cadvisor but the fix is trivial (google/cadvisor@48e7375#diff-c1ad83c3e8484f4f134e3fb0eff9666f7b19db23d0c20bd5476920ff1bcd9709)

@kolyshkin Thank you for fixing this.

By the way @xiaochenshen @Creatone why do we have to specify empty config, like this:

mgr := intelrdt.NewManager(&configs.Config{IntelRdt: &configs.IntelRdt{}}, id, resctrlPath)

@kolyshkin The reason why we have to specify empty config:
In #1615 fixing, we add check if Config.IntelRdt == nil to make sure that GetStats() doesn't return error unexpectedly in this

runc/libcontainer/intelrdt/intelrdt.go

Lines 587 to 591 in 27227a9

	func (m *intelRdtManager) GetStats() (*Stats, error) {
	// If intelRdt is not specified in config
	if m.config.IntelRdt == nil {
	return nil, nil
	}

special case in runc update:
If intelRdt is not specified in original configuration when starting the container, we could update L3CacheSchema and MemBwSchema in config.IntelRdt through runc update command later in runtime.

See the code and comment here:

runc/update.go

Lines 310 to 328 in 27227a9

	if l3CacheSchema != "" || memBwSchema != "" {
	// If intelRdt is not specified in original configuration, we just don't
	// Apply() to create intelRdt group or attach tasks for this container.
	// In update command, we could re-enable through IntelRdtManager.Apply()
	// and then update intelrdt constraint.
	if config.IntelRdt == nil {
	state, err := container.State()
	if err != nil {
	return err
	}
	config.IntelRdt = &configs.IntelRdt{}
	intelRdtManager := intelrdt.NewManager(&config, container.ID(), state.IntelRdtPath)
	if err := intelRdtManager.Apply(state.InitProcessPid); err != nil {
	return err
	}
	}
	config.IntelRdt.L3CacheSchema = l3CacheSchema
	config.IntelRdt.MemBwSchema = memBwSchema
	}

Can we do something like

Yes. I think so.

func NewManager(config *configs.Config, id string, path string) Manager {
        if config == nil {

Maybe:
if config.IntelRdt == nil { ?

                config = &configs.Config{IntelRdt: &configs.IntelRdt{}}
        }
        return &intelRdtManager{
                config: config,
                id:     id,
                path:   path,
        }
}

or it does not make any sense?