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.

Sign up for GitHub

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

Jump to bottom

HTCONDOR-2389 startd-string-lists #2421

Merged
merged 2 commits into from
Apr 26, 2024
Merged

HTCONDOR-2389 startd-string-lists #2421

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3bb5bf2
HTCONDOR-2389 Convert StringLists in the startd
JaimeFrey Apr 26, 2024
788dd59
HTCONDOR-2389 range-for-loop-ify ResAttributes
GregThain Apr 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
129 changes: 62 additions & 67 deletions src/condor_startd.V6/ResAttributes.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
#include "docker-api.h"

MachAttributes::MachAttributes()
: m_user_specified(NULL, ";"), m_user_settings_init(false), m_named_chroot()
: m_user_settings_init(false), m_named_chroot()
{
m_mips = -1;
m_kflops = -1;
Expand Down Expand Up @@ -171,7 +171,6 @@ MachAttributes::~MachAttributes()
}
m_lst_static.clear();

m_user_specified.clearAll();
#if defined(WIN32)
if( m_local_credd ) free( m_local_credd );
if( m_dot_Net_Versions ) free ( m_dot_Net_Versions );
Expand All @@ -195,40 +194,39 @@ MachAttributes::init_user_settings()
}
m_lst_static.clear();

m_user_specified.clearAll();
m_user_specified.clear();

#ifdef WIN32
char * pszParam = NULL;
pszParam = param("STARTD_PUBLISH_WINREG");
if (pszParam)
{
m_user_specified.initializeFromString(pszParam);
m_user_specified = split(pszParam, ";");
free(pszParam);
}
#endif

m_user_specified.rewind();
while(char * pszItem = m_user_specified.next())
for (const auto& pszItem: m_user_specified)
{
// if the reg_item is of the form attr_name=reg_path;
// then skip over the attr_name and '=' and trailing
// whitespace. But if the = is after the first \, then
// it's a part of the reg_path, so ignore it.
//
const char * pkey = strchr(pszItem, '=');
const char * pbs = strchr(pszItem, '\\');
const char * pkey = strchr(pszItem.c_str(), '=');
const char * pbs = strchr(pszItem.c_str(), '\\');
if (pkey && ( ! pbs || pkey < pbs)) {
++pkey; // skip the '='
} else {
pkey = pszItem;
pkey = pszItem.c_str();
}

// skip any leading whitespace in the key
while (isspace(pkey[0]))
++pkey;

#ifdef WIN32
char * pszAttr = generate_reg_key_attr_name("WINREG_", pszItem);
char * pszAttr = generate_reg_key_attr_name("WINREG_", pszItem.c_str());

// if the keyname begins with 32 or 64, use that to designate either
// the WOW64 or WOW32 view of the registry, but don't pass the
Expand Down Expand Up @@ -935,9 +933,9 @@ bool MachAttributes::ComputeDevProps(
// fungable resource, or a list of ids of non-fungable resources.
// if res_value contains a list, then ids is set on exit from this function
// otherwise, ids empty.
static double parse_user_resource_config(const char * tag, const char * res_value, StringList & ids)
static double parse_user_resource_config(const char * tag, const char * res_value, std::set<std::string> & ids)
{
ids.clearAll();
ids.clear();
double num = 0;
char * pend = NULL;
num = strtod(res_value, &pend);
Expand All @@ -951,8 +949,10 @@ static double parse_user_resource_config(const char * tag, const char * res_valu
is_simple_double = true;
} else {
// not a simple double, so treat it as a list of id's, the number of resources is the number of ids
ids.initializeFromString(res_value);
num = ids.number();
for (const auto& item: StringTokenIterator(res_value)) {
ids.insert(item);
}
num = ids.size();
}
}

Expand Down Expand Up @@ -989,7 +989,7 @@ double MachAttributes::init_machine_resource_from_script(const char * tag, const
classad::Value value;
std::string attr(ATTR_OFFLINE_PREFIX); attr += tag;
std::string res_value;
StringList offline_ids;
std::set<std::string> offline_ids;
if (ad.LookupString(attr,res_value)) {
offline = parse_user_resource_config(tag, res_value.c_str(), offline_ids);
} else {
Expand All @@ -1001,13 +1001,12 @@ double MachAttributes::init_machine_resource_from_script(const char * tag, const

attr = ATTR_DETECTED_PREFIX; attr += tag;
if (ad.LookupString(attr,res_value)) {
StringList ids;
std::set<std::string> ids;
quantity = parse_user_resource_config(tag, res_value.c_str(), ids);
if ( ! ids.isEmpty()) {
if ( ! ids.empty()) {
offline = 0; // we only want to count ids that match the detected list
ids.rewind();
while (const char* id = ids.next()) {
if (offline_ids.contains(id)) {
for (const auto& id: ids) {
if (offline_ids.count(id)) {
unique_ids.insert(id);
this->m_machres_offline_devIds_map[tag].emplace_back(id);
++offline;
Expand Down Expand Up @@ -1036,7 +1035,7 @@ double MachAttributes::init_machine_resource_from_script(const char * tag, const
// then remove it from the nested ad collection so we don't see it when we iterate
auto common_it = nested.find("common");
if (common_it != nested.end()) {
for (auto id : unique_ids) {
for (const auto& id : unique_ids) {
m_machres_nft_map[tag].props[id].Update(*common_it->second);
}
ad.Delete(common_it->first);
Expand Down Expand Up @@ -1097,21 +1096,20 @@ bool MachAttributes::init_machine_resource(MachAttributes * pme, HASHITER & it)
num = pme->init_machine_resource_from_script(tag, res_value);
}
} else {
StringList offline_ids;
std::set<std::string> offline_ids;
std::string off_name("OFFLINE_"); off_name += name;
std::string my_value;
if (param(my_value, off_name.c_str()) && !my_value.empty()) {
offline = parse_user_resource_config(tag, my_value.c_str(), offline_ids);
}

// if the param value parses as a double, then we can handle it simply
StringList ids;
std::set<std::string> ids;
num = parse_user_resource_config(tag, res_value, ids);
if ( ! ids.isEmpty()) {
if ( ! ids.empty()) {
offline = 0; // we only want to count ids that match the detected list
ids.rewind();
while (const char* id = ids.next()) {
if (offline_ids.contains(id)) {
for (const auto& id: ids) {
if (offline_ids.count(id)) {
pme->m_machres_offline_devIds_map[tag].emplace_back(id);
++offline;
} else {
Expand Down Expand Up @@ -1163,30 +1161,30 @@ void MachAttributes::init_machine_resources() {
const int iter_options = HASHITER_NO_DEFAULTS; // we can speed up iteration if there are no machine resources in the defaults table.
foreach_param_matching(re, iter_options, (bool(*)(void*,HASHITER&))MachAttributes::init_machine_resource, this);

StringList allowed;
std::vector<std::string> allowed;
char* allowed_names = param("MACHINE_RESOURCE_NAMES");
if (allowed_names && allowed_names[0]) {
if (allowed_names) {
// if admin defined MACHINE_RESOURCE_NAMES, then use this list
// as the source of expected custom machine resources
allowed.initializeFromString(allowed_names);
allowed = split(allowed_names);
free(allowed_names);
}

StringList disallowed("CPU CPUS DISK SWAP MEM MEMORY RAM");
std::vector<std::string> disallowed = {"CPU", "CPUS", "DISK", "SWAP", "MEM", "MEMORY", "RAM"};

std::vector<const char *> tags_to_erase;
for (slotres_map_t::iterator it(m_machres_map.begin()); it != m_machres_map.end(); ++it) {
const char * tag = it->first.c_str();
if ( ! allowed.isEmpty() && ! allowed.contains_anycase(tag)) {
for (auto & it : m_machres_map) {
const char * tag = it.first.c_str();
if ( ! allowed.empty() && ! contains_anycase(allowed, tag)) {
dprintf(D_ALWAYS, "Local machine resource %s is not in MACHINE_RESOURCE_NAMES so it will have no effect\n", tag);
tags_to_erase.emplace_back(tag);
continue;
}
if ( ! disallowed.isEmpty() && disallowed.contains_anycase(tag)) {
if ( ! disallowed.empty() && contains_anycase(disallowed, tag)) {
EXCEPT("fatal error - MACHINE_RESOURCE_%s is invalid, '%s' is a reserved resource name", tag, tag);
continue;
}
dprintf(D_ALWAYS, "Local machine resource %s = %g\n", tag, it->second);
dprintf(D_ALWAYS, "Local machine resource %s = %g\n", tag, it.second);
}
for( auto i = tags_to_erase.begin(); i != tags_to_erase.end(); ++i ) {
m_machres_map.erase(*i);
Expand Down Expand Up @@ -1379,23 +1377,23 @@ MachAttributes::publish_static(ClassAd* cp)

std::string machine_resources = "Cpus Memory Disk Swap";
// publish any local resources
for (slotres_map_t::iterator j(m_machres_map.begin()); j != m_machres_map.end(); ++j) {
const char * rname = j->first.c_str();
for (auto & j : m_machres_map) {
const char * rname = j.first.c_str();
std::string attr(ATTR_DETECTED_PREFIX); attr += rname;
double ipart, fpart = modf(j->second, &ipart);
double ipart, fpart = modf(j.second, &ipart);
if (fpart >= 0.0 && fpart <= 0.0) {
cp->Assign(attr, (long long)ipart);
} else {
cp->Assign(attr, j->second);
cp->Assign(attr, j.second);
}
attr = ATTR_TOTAL_PREFIX; attr += rname;
if (fpart >= 0.0 && fpart <= 0.0) {
cp->Assign(attr, (long long)ipart);
} else {
cp->Assign(attr, j->second);
cp->Assign(attr, j.second);
}
machine_resources += " ";
machine_resources += j->first;
machine_resources += j.first;
}
cp->Assign(ATTR_MACHINE_RESOURCES, machine_resources);

Expand All @@ -1407,10 +1405,7 @@ MachAttributes::publish_static(ClassAd* cp)
// Advertise Docker Volumes
char *dockerVolumes = param("DOCKER_VOLUMES");
if (dockerVolumes) {
StringList vl(dockerVolumes);
vl.rewind();
char *volume = 0;
while ((volume = vl.next())) {
for (const auto& volume: StringTokenIterator(dockerVolumes)) {
std::string attrName = "HasDockerVolume";
attrName += volume;
cp->Assign(attrName, true);
Expand Down Expand Up @@ -1885,36 +1880,36 @@ CpuAttributes::bind_DevIds(MachAttributes* map, int slot_id, int slot_sub_id, bo
}
}

for (slotres_map_t::iterator j(c_slotres_map.begin()); j != c_slotres_map.end(); ++j) {
for (auto & j : c_slotres_map) {
// TODO: handle fractional assigned custome resources?
int cAssigned = int(j->second);
int cAssigned = int(j.second);

// if this resource already has bound ids, don't bind again.
slotres_devIds_map_t::const_iterator m(c_slotres_ids_map.find(j->first));
slotres_devIds_map_t::const_iterator m(c_slotres_ids_map.find(j.first));
if (m != c_slotres_ids_map.end() && cAssigned == (int)m->second.size())
continue;

const char * request = nullptr;
slotres_constraint_map_t::const_iterator req(c_slotres_constraint_map.find(j->first));
slotres_constraint_map_t::const_iterator req(c_slotres_constraint_map.find(j.first));
if (req != c_slotres_constraint_map.end()) { request = req->second.c_str(); }

::dprintf(D_ALWAYS, "bind %s DevIds tag=%s contraint=%s\n", name, j->first.c_str(), request ? request : "");
::dprintf(D_ALWAYS, "bind %s DevIds tag=%s contraint=%s\n", name, j.first.c_str(), request ? request : "");

if (map->machres_devIds().count(j->first)) {
if (map->machres_devIds().count(j.first)) {
int cAllocated = 0;
for (int ii = 0; ii < cAssigned; ++ii) {
const char * id = map->AllocateDevId(j->first, request, slot_id, slot_sub_id, backfill_slot);
const char * id = map->AllocateDevId(j.first, request, slot_id, slot_sub_id, backfill_slot);
if (id) {
++cAllocated;
c_slotres_ids_map[j->first].push_back(id);
c_slotres_ids_map[j.first].push_back(id);
::dprintf(d_log_devids, "bind DevIds for %s bound %s %d\n",
name, id, (int)c_slotres_ids_map[j->first].size());
name, id, (int)c_slotres_ids_map[j.first].size());
}
}
if (cAllocated < cAssigned) {
::dprintf(D_ALWAYS | D_BACKTRACE, "%s Failed to bind local resource '%s'\n", name, j->first.c_str());
::dprintf(D_ALWAYS | D_BACKTRACE, "%s Failed to bind local resource '%s'\n", name, j.first.c_str());
if (abort_on_fail) {
EXCEPT("Failed to bind local resource '%s'", j->first.c_str());
EXCEPT("Failed to bind local resource '%s'", j.first.c_str());
}
return false;
}
Expand All @@ -1923,8 +1918,8 @@ CpuAttributes::bind_DevIds(MachAttributes* map, int slot_id, int slot_sub_id, bo
// of making c_slot_res_ids_map['tag'] exist, which in turn results in the
// "Assigned<Tag>" slot attribute being defined rather than undefined.
if (cAllocated > 0) {
c_slotres_props_map[j->first].Clear();
map->ComputeDevProps(c_slotres_props_map[j->first], j->first, c_slotres_ids_map[j->first]);
c_slotres_props_map[j.first].Clear();
map->ComputeDevProps(c_slotres_props_map[j.first], j.first, c_slotres_ids_map[j.first]);
}
}
}
Expand Down Expand Up @@ -1953,12 +1948,12 @@ CpuAttributes::unbind_DevIds(MachAttributes* map, int slot_id, int slot_sub_id)

if ( ! slot_sub_id) return;

for (slotres_map_t::iterator j(c_slotres_map.begin()); j != c_slotres_map.end(); ++j) {
slotres_devIds_map_t::const_iterator k(c_slotres_ids_map.find(j->first));
for (auto & j : c_slotres_map) {
slotres_devIds_map_t::const_iterator k(c_slotres_ids_map.find(j.first));
if (k != c_slotres_ids_map.end()) {
slotres_assigned_ids_t & ids = c_slotres_ids_map[j->first];
slotres_assigned_ids_t & ids = c_slotres_ids_map[j.first];
while ( ! ids.empty()) {
bool released = map->ReleaseDynamicDevId(j->first, ids.back().c_str(), slot_id, slot_sub_id);
bool released = map->ReleaseDynamicDevId(j.first, ids.back().c_str(), slot_id, slot_sub_id);
dprintf(released ? d_log_devids : D_ALWAYS, "ubind DevIds for slot%d.%d unbind %s %d %s\n",
slot_id, slot_sub_id, ids.back().c_str(), (int)ids.size(), released ? "OK" : "failed");
ids.pop_back();
Expand Down Expand Up @@ -2394,8 +2389,8 @@ AvailAttributes::AvailAttributes( MachAttributes* map ):
a_virt_mem_fraction = 1.0;
a_virt_mem_auto_count = 0;
a_slotres_map = map->machres();
for (slotres_map_t::iterator j(a_slotres_map.begin()); j != a_slotres_map.end(); ++j) {
a_autocnt_map[j->first] = 0;
for (auto & j : a_slotres_map) {
a_autocnt_map[j.first] = 0;
}
}

Expand Down Expand Up @@ -2595,7 +2590,7 @@ AvailAttributes::cat_totals(std::string & buf, const char * execute_partition_id
formatstr_cat(buf, "Cpus: %d, Memory: %d, Swap: %.2f%%, Disk: %.2f%%",
a_num_cpus, a_phys_mem, 100*a_virt_mem_fraction,
100*partition.m_disk_fraction );
for (slotres_map_t::iterator j(a_slotres_map.begin()); j != a_slotres_map.end(); ++j) {
formatstr_cat(buf, ", %s: %d", j->first.c_str(), int(j->second));
for (auto & j : a_slotres_map) {
formatstr_cat(buf, ", %s: %d", j.first.c_str(), int(j.second));
}
}
4 changes: 2 additions & 2 deletions src/condor_startd.V6/ResAttributes.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,7 @@ class MachAttributes
// to initialize m_lst_static and m_lst_dynamic. these two lists
// continue to hold pointers into this. do not free it unless you first empty those
// lists. -tj
StringList m_user_specified;
std::vector<std::string> m_user_specified;
int m_user_settings_init; // set to true when init_user_settings has been called at least once.

std::string m_named_chroot;
Expand Down Expand Up @@ -422,7 +422,7 @@ class CpuAttributes
const std::string &execute_dir, const std::string &execute_partition_id );

// init a slot_request from config strings
static bool buildSlotRequest(_slot_request & request, MachAttributes *m_attr, StringList* list, unsigned int type_id, bool except);
static bool buildSlotRequest(_slot_request & request, MachAttributes *m_attr, const std::string& list, unsigned int type_id, bool except);

// construct from a _slot_request
CpuAttributes(unsigned int slot_type,
Expand Down