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

Correctly obtain name Ceph service names #9

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 14 additions & 9 deletions pkg/virt/virt.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,22 +15,22 @@ import (
"path/filepath"
"strings"

"github.com/pkg/errors"
"github.com/infrawatch/collectd-libpod-stats/pkg/cgroups"
"github.com/infrawatch/collectd-libpod-stats/pkg/containers"
"github.com/pkg/errors"
)

const (
//container path relative to user
relativeContainersPath string = "containers/storage/overlay-containers/containers.json"
relativeContainersPath string = "containers/storage/overlay-containers/containers.json"
relativeVolatileContainersPath string = "containers/storage/overlay-containers/volatile-containers.json"
)

//MetricMatrix holds stats for each container according to
//control type. Usage is: map[container label]map[control type]data
// MetricMatrix holds stats for each container according to
// control type. Usage is: map[container label]map[control type]data
type MetricMatrix map[string]map[cgroups.ControlType]uint64

//ContainersStats retrieves stats in specified cgroup controllers for all containers on host
// ContainersStats retrieves stats in specified cgroup controllers for all containers on host
func ContainersStats(cgroupControls ...cgroups.ControlType) (MetricMatrix, error) {
cgroup2, err := cgroups.IsCgroup2UnifiedMode()
if err != nil {
Expand Down Expand Up @@ -71,7 +71,7 @@ func ContainersStats(cgroupControls ...cgroups.ControlType) (MetricMatrix, error
return retMatrix, nil
}

//getContainers returns map with containers created on host indexed by name
// getContainers returns map with containers created on host indexed by name
func getContainers() (map[string]*containers.Container, error) {
/*
libpod stores container related information in one of two places:
Expand Down Expand Up @@ -173,9 +173,14 @@ func genContainerCgroupPath(ctype cgroups.ControlType, id string, cgroup2 bool,
node_hostname = strings.Split(node_hostname, ".")[0]

// from `container_name` get the name of the ceph service for which cgroup path is to be found
ceph_service_name := strings.Split(container_name, node_hostname)[0]
ceph_service_name = strings.Trim(ceph_service_name, "-")
ceph_service_name = strings.SplitAfter(ceph_service_name, fmt.Sprintf("ceph-%s-", ceph_fsid))[1]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ceph_service_name is not used afterwards anymore, in that case it can completely go away?

Copy link
Contributor Author

@yadneshk yadneshk May 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realized using ceph_service_name improves code readability, so I've reintroduced the variable. In the latest changes, we obtain ceph service's name from the container name.
This change is needed because of https://bugzilla.redhat.com/show_bug.cgi?id=2279805

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might have changed after the comment, but right now ceph_service_name is used at L191.

if strings.Contains(container_name, node_hostname) {
container_name = strings.Split(container_name, node_hostname)[0]
}

if strings.Contains(container_name, fmt.Sprintf("ceph-%s", ceph_fsid)) {
container_name = strings.Split(container_name, fmt.Sprintf("ceph-%s", ceph_fsid))[1]
}
ceph_service_name := strings.Trim(container_name, "-")

/*
Find out the directory starting with the prefix "ceph-<FSID>-" corresponding to `ceph_service_name`
Expand Down