From 0a7ee5b825c5ee3859c8dbc7a2c2a8dce1168778 Mon Sep 17 00:00:00 2001 From: Jake Smith Date: Thu, 28 Sep 2023 10:05:41 +0100 Subject: [PATCH 1/2] HPCC-30390 Improve get dafilesrv service errors Signed-off-by: Jake Smith --- system/jlib/jerror.hpp | 1 + system/jlib/jmisc.cpp | 8 ++++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/system/jlib/jerror.hpp b/system/jlib/jerror.hpp index 1ab11623d89..264c33ed3a3 100644 --- a/system/jlib/jerror.hpp +++ b/system/jlib/jerror.hpp @@ -33,6 +33,7 @@ #define JLIBERR_InternalError 6002 #define JLIBERR_CppCompileError 6003 #define JLIBERR_UnexpectedValue 6004 +#define JLIBERR_K8sServiceError 6005 //---- Text for all errors (make it easy to internationalise) --------------------------- diff --git a/system/jlib/jmisc.cpp b/system/jlib/jmisc.cpp index 994d691b1c2..6fa8e1f8e35 100644 --- a/system/jlib/jmisc.cpp +++ b/system/jlib/jmisc.cpp @@ -1084,17 +1084,17 @@ std::pair getDafileServiceFromConfig(const char *applicat VStringBuffer serviceXPath("services[@type='%s']", application); Owned dafilesrvServices = getGlobalConfigSP()->getElements(serviceXPath); if (!dafilesrvServices->first()) - throw makeStringException(-1, "dafilesrv service not defined"); + throw makeStringExceptionV(JLIBERR_K8sServiceError, "dafilesrv service '%s' not defined or disabled", application); const IPropertyTree &dafilesrv = dafilesrvServices->query(); if (!dafilesrv.getPropBool("@public")) - throw makeStringException(-1, "dafilesrv service has no public service defined"); + throw makeStringExceptionV(JLIBERR_K8sServiceError, "dafilesrv service '%s' has no public service defined", application); StringBuffer dafilesrvName; dafilesrv.getProp("@name", dafilesrvName); auto externalService = getExternalService(dafilesrvName); if (externalService.first.empty()) - throw makeStringExceptionV(-1, "dafilesrv '%s': external service not found", dafilesrvName.str()); + throw makeStringExceptionV(JLIBERR_K8sServiceError, "dafilesrv service '%s' - external service '%s' not found", application, dafilesrvName.str()); if (0 == externalService.second) - throw makeStringExceptionV(-1, "dafilesrv '%s': external service port not defined", dafilesrvName.str()); + throw makeStringExceptionV(JLIBERR_K8sServiceError, "dafilesrv service '%s' - external service '%s' port not defined", application, dafilesrvName.str()); return externalService; } From 42fbde6d55047a298fabd7f63a692af2a4924c5a Mon Sep 17 00:00:00 2001 From: Jake Smith Date: Thu, 28 Sep 2023 10:24:12 +0100 Subject: [PATCH 2/2] HPCC-30390 move k8s functions into jcontainerized Signed-off-by: Jake Smith --- dali/base/dautils.cpp | 3 +- esp/clients/ws_dfsclient/ws_dfsclient.cpp | 3 +- esp/services/ws_dfu/ws_dfuService.cpp | 5 +- system/jlib/jcontainerized.cpp | 96 +++++++++++++++++++++++ system/jlib/jcontainerized.hpp | 7 ++ system/jlib/jmisc.cpp | 94 ---------------------- system/jlib/jmisc.hpp | 8 -- 7 files changed, 110 insertions(+), 106 deletions(-) diff --git a/dali/base/dautils.cpp b/dali/base/dautils.cpp index f14a8df714a..fe3eb7fcb0a 100644 --- a/dali/base/dautils.cpp +++ b/dali/base/dautils.cpp @@ -17,6 +17,7 @@ #include "platform.h" #include "jlib.hpp" +#include "jcontainerized.hpp" #include "jstring.hpp" #include "jfile.hpp" #include "jmisc.hpp" @@ -3628,7 +3629,7 @@ void remapGroupsToDafilesrv(IPropertyTree *file, INamedGroupStore *resolver) CriticalBlock b(dafileSrvNodeCS); if (nullptr == dafileSrvNode) { - auto externalService = getDafileServiceFromConfig("directio"); + auto externalService = k8s::getDafileServiceFromConfig("directio"); VStringBuffer dafilesrvEpStr("%s:%u", externalService.first.c_str(), externalService.second); dafileSrvNode.setown(createINode(dafilesrvEpStr)); } diff --git a/esp/clients/ws_dfsclient/ws_dfsclient.cpp b/esp/clients/ws_dfsclient/ws_dfsclient.cpp index a1d0f544f2d..ad0c2f37fd5 100644 --- a/esp/clients/ws_dfsclient/ws_dfsclient.cpp +++ b/esp/clients/ws_dfsclient/ws_dfsclient.cpp @@ -18,6 +18,7 @@ #include #include "jliball.hpp" +#include "jcontainerized.hpp" #include "jflz.hpp" #include "jsecrets.hpp" #include "seclib.hpp" @@ -726,7 +727,7 @@ IDFSFile *lookupDFSFile(const char *logicalName, AccessMode accessMode, unsigned const IPropertyTree &eclWatch = eclWatchServices->query(); StringBuffer eclWatchName; eclWatch.getProp("@name", eclWatchName); - auto result = getExternalService(eclWatchName); + auto result = k8s::getExternalService(eclWatchName); if (result.first.empty()) throw makeStringExceptionV(-1, "dfs '%s': service not found", eclWatchName.str()); if (0 == result.second) diff --git a/esp/services/ws_dfu/ws_dfuService.cpp b/esp/services/ws_dfu/ws_dfuService.cpp index 4dcfd29b947..068394cc171 100644 --- a/esp/services/ws_dfu/ws_dfuService.cpp +++ b/esp/services/ws_dfu/ws_dfuService.cpp @@ -19,6 +19,7 @@ #include +#include "jcontainerized.hpp" #include "daclient.hpp" #include "daft.hpp" #include "daftcfg.hpp" @@ -6116,7 +6117,7 @@ void CWsDfuEx::dFUFileAccessCommon(IEspContext &context, const CDfsLogicalFileNa if (!info) throw makeStringExceptionV(-1, "dFUFileAccessCommon: file signing certificate ('%s') not defined in configuration.", keyPairName.str()); - auto externalService = getDafileServiceFromConfig("stream"); + auto externalService = k8s::getDafileServiceFromConfig("stream"); dafilesrvHost.set(externalService.first.c_str()); port = externalService.second; secure = true; @@ -6498,7 +6499,7 @@ bool CWsDfuEx::onDFUFileCreateV2(IEspContext &context, IEspDFUFileCreateV2Reques fileDesc.setown(createFileDescriptor(tempFileName, planeName, numParts)); numParts = fileDesc->numParts(); - auto externalService = getDafileServiceFromConfig("stream"); + auto externalService = k8s::getDafileServiceFromConfig("stream"); dafilesrvHost.set(externalService.first.c_str()); port = externalService.second; secure = true; diff --git a/system/jlib/jcontainerized.cpp b/system/jlib/jcontainerized.cpp index 093e647834b..3b9724e2682 100644 --- a/system/jlib/jcontainerized.cpp +++ b/system/jlib/jcontainerized.cpp @@ -11,6 +11,7 @@ limitations under the License. ############################################################################## */ +#include "jerror.hpp" #include "jexcept.hpp" #include "jfile.hpp" #include "jmisc.hpp" @@ -292,6 +293,101 @@ std::vector> getPodNodes(const char *selector) } } +void runKubectlCommand(const char *title, const char *cmd, const char *input, StringBuffer *output) +{ +#ifndef _CONTAINERIZED + UNIMPLEMENTED_X("runKubectlCommand"); +#endif +// NB: will fire an exception if command fails (returns non-zero exit code) + + StringBuffer _output, error; + if (!output) + output = &_output; + unsigned ret = runExternalCommand(title, *output, error, cmd, input, ".", nullptr); + if (output->length()) + MLOG(MCdebugInfo, unknownJob, "%s: ret=%u, stdout=%s", cmd, ret, output->trimRight().str()); + if (error.length()) + MLOG(MCdebugError, unknownJob, "%s: ret=%u, stderr=%s", cmd, ret, error.trimRight().str()); + if (ret) + { + if (input) + MLOG(MCdebugError, unknownJob, "Using input %s", input); + throw makeStringExceptionV(0, "Failed to run %s: error %u: %s", cmd, ret, error.str()); + } +} + +static CTimeLimitedCache> externalServiceCache; +static CriticalSection externalServiceCacheCrit; +std::pair getExternalService(const char *serviceName) +{ +#ifndef _CONTAINERIZED + UNIMPLEMENTED_X("getExternalService"); +#endif + { + CriticalBlock b(externalServiceCacheCrit); + std::pair cachedExternalSevice; + if (externalServiceCache.get(serviceName, cachedExternalSevice)) + return cachedExternalSevice; + } + + StringBuffer output; + try + { + VStringBuffer getServiceCmd("kubectl get svc --selector=server=%s --output=jsonpath={.items[0].status.loadBalancer.ingress[0].hostname},{.items[0].status.loadBalancer.ingress[0].ip},{.items[0].spec.ports[0].port}", serviceName); + k8s::runKubectlCommand("get-external-service", getServiceCmd, nullptr, &output); + } + catch (IException *e) + { + EXCLOG(e); + VStringBuffer exceptionText("Failed to get external service for '%s'. Error: [%d, ", serviceName, e->errorCode()); + e->errorMessage(exceptionText).append("]"); + e->Release(); + throw makeStringException(-1, exceptionText); + } + StringArray fields; + fields.appendList(output, ","); + + // NB: add even if no result, want non-result to be cached too + std::string host, port; + if (fields.ordinality() == 3) // hostname,ip,port. NB: hostname may be missing, but still present as a blank field + { + host = fields.item(0); // hostname + if (0 == host.length()) + host = fields.item(1); // ip + port = fields.item(2); + } + auto servicePair = std::make_pair(host, atoi(port.c_str())); + externalServiceCache.add(serviceName, servicePair); + return servicePair; +} + +std::pair getDafileServiceFromConfig(const char *application) +{ +#ifndef _CONTAINERIZED + UNIMPLEMENTED_X("getDafileServiceFromConfig"); +#endif + /* NB: For now expect 1 dafilesrv in configuration only + * We could have multiple dafilesrv services with e.g. different specs./replicas etc. that + * serviced different planes. At the moment dafilesrv mounts all data planes. + */ + VStringBuffer serviceXPath("services[@type='%s']", application); + Owned dafilesrvServices = getGlobalConfigSP()->getElements(serviceXPath); + if (!dafilesrvServices->first()) + throw makeStringExceptionV(JLIBERR_K8sServiceError, "dafilesrv service '%s' not defined or disabled", application); + const IPropertyTree &dafilesrv = dafilesrvServices->query(); + if (!dafilesrv.getPropBool("@public")) + throw makeStringExceptionV(JLIBERR_K8sServiceError, "dafilesrv service '%s' has no public service defined", application); + StringBuffer dafilesrvName; + dafilesrv.getProp("@name", dafilesrvName); + auto externalService = getExternalService(dafilesrvName); + if (externalService.first.empty()) + throw makeStringExceptionV(JLIBERR_K8sServiceError, "dafilesrv service '%s' - external service '%s' not found", application, dafilesrvName.str()); + if (0 == externalService.second) + throw makeStringExceptionV(JLIBERR_K8sServiceError, "dafilesrv service '%s' - external service '%s' port not defined", application, dafilesrvName.str()); + return externalService; +} + + static unsigned podInfoInitCBId = 0; MODULE_INIT(INIT_PRIORITY_STANDARD) { diff --git a/system/jlib/jcontainerized.hpp b/system/jlib/jcontainerized.hpp index cb8100791c8..de942b9636d 100644 --- a/system/jlib/jcontainerized.hpp +++ b/system/jlib/jcontainerized.hpp @@ -37,5 +37,12 @@ jlib_decl void waitJob(const char *componentName, const char *job, unsigned pend jlib_decl bool applyYaml(const char *componentName, const char *wuid, const char *job, const char *resourceType, const std::list> &extraParams, bool optional, bool autoCleanup); jlib_decl void runJob(const char *componentName, const char *wuid, const char *job, const std::list> &extraParams={}); +extern jlib_decl void runKubectlCommand(const char *title, const char *cmd, const char *input, StringBuffer *output); + +// return the k8s external host and port for serviceName +extern jlib_decl std::pair getExternalService(const char *serviceName); + +extern jlib_decl std::pair getDafileServiceFromConfig(const char *application); + } diff --git a/system/jlib/jmisc.cpp b/system/jlib/jmisc.cpp index 6fa8e1f8e35..f6560ec9967 100644 --- a/system/jlib/jmisc.cpp +++ b/system/jlib/jmisc.cpp @@ -1004,100 +1004,6 @@ jlib_decl char **getSystemEnv() } -void runKubectlCommand(const char *title, const char *cmd, const char *input, StringBuffer *output) -{ -#ifndef _CONTAINERIZED - UNIMPLEMENTED_X("runKubectlCommand"); -#endif -// NB: will fire an exception if command fails (returns non-zero exit code) - - StringBuffer _output, error; - if (!output) - output = &_output; - unsigned ret = runExternalCommand(title, *output, error, cmd, input, ".", nullptr); - if (output->length()) - MLOG(MCdebugInfo, unknownJob, "%s: ret=%u, stdout=%s", cmd, ret, output->trimRight().str()); - if (error.length()) - MLOG(MCdebugError, unknownJob, "%s: ret=%u, stderr=%s", cmd, ret, error.trimRight().str()); - if (ret) - { - if (input) - MLOG(MCdebugError, unknownJob, "Using input %s", input); - throw makeStringExceptionV(0, "Failed to run %s: error %u: %s", cmd, ret, error.str()); - } -} - -static CTimeLimitedCache> externalServiceCache; -static CriticalSection externalServiceCacheCrit; -std::pair getExternalService(const char *serviceName) -{ -#ifndef _CONTAINERIZED - UNIMPLEMENTED_X("getExternalService"); -#endif - { - CriticalBlock b(externalServiceCacheCrit); - std::pair cachedExternalSevice; - if (externalServiceCache.get(serviceName, cachedExternalSevice)) - return cachedExternalSevice; - } - - StringBuffer output; - try - { - VStringBuffer getServiceCmd("kubectl get svc --selector=server=%s --output=jsonpath={.items[0].status.loadBalancer.ingress[0].hostname},{.items[0].status.loadBalancer.ingress[0].ip},{.items[0].spec.ports[0].port}", serviceName); - runKubectlCommand("get-external-service", getServiceCmd, nullptr, &output); - } - catch (IException *e) - { - EXCLOG(e); - VStringBuffer exceptionText("Failed to get external service for '%s'. Error: [%d, ", serviceName, e->errorCode()); - e->errorMessage(exceptionText).append("]"); - e->Release(); - throw makeStringException(-1, exceptionText); - } - StringArray fields; - fields.appendList(output, ","); - - // NB: add even if no result, want non-result to be cached too - std::string host, port; - if (fields.ordinality() == 3) // hostname,ip,port. NB: hostname may be missing, but still present as a blank field - { - host = fields.item(0); // hostname - if (0 == host.length()) - host = fields.item(1); // ip - port = fields.item(2); - } - auto servicePair = std::make_pair(host, atoi(port.c_str())); - externalServiceCache.add(serviceName, servicePair); - return servicePair; -} - -std::pair getDafileServiceFromConfig(const char *application) -{ -#ifndef _CONTAINERIZED - UNIMPLEMENTED_X("getDafileServiceFromConfig"); -#endif - /* NB: For now expect 1 dafilesrv in configuration only - * We could have multiple dafilesrv services with e.g. different specs./replicas etc. that - * serviced different planes. At the moment dafilesrv mounts all data planes. - */ - VStringBuffer serviceXPath("services[@type='%s']", application); - Owned dafilesrvServices = getGlobalConfigSP()->getElements(serviceXPath); - if (!dafilesrvServices->first()) - throw makeStringExceptionV(JLIBERR_K8sServiceError, "dafilesrv service '%s' not defined or disabled", application); - const IPropertyTree &dafilesrv = dafilesrvServices->query(); - if (!dafilesrv.getPropBool("@public")) - throw makeStringExceptionV(JLIBERR_K8sServiceError, "dafilesrv service '%s' has no public service defined", application); - StringBuffer dafilesrvName; - dafilesrv.getProp("@name", dafilesrvName); - auto externalService = getExternalService(dafilesrvName); - if (externalService.first.empty()) - throw makeStringExceptionV(JLIBERR_K8sServiceError, "dafilesrv service '%s' - external service '%s' not found", application, dafilesrvName.str()); - if (0 == externalService.second) - throw makeStringExceptionV(JLIBERR_K8sServiceError, "dafilesrv service '%s' - external service '%s' port not defined", application, dafilesrvName.str()); - return externalService; -} - // checks if 'name' is an internal environment variable (prefixed with 'HPCC_') // if !matches : returns null // if matches : returns allocated copy of configured value or defaultValue if not set. diff --git a/system/jlib/jmisc.hpp b/system/jlib/jmisc.hpp index 1943404871a..bc3cdd07205 100644 --- a/system/jlib/jmisc.hpp +++ b/system/jlib/jmisc.hpp @@ -332,14 +332,6 @@ extern jlib_decl char *mkdtemp(char *_template); extern jlib_decl char **getSystemEnv(); - -extern jlib_decl void runKubectlCommand(const char *title, const char *cmd, const char *input, StringBuffer *output); - -// return the k8s external host and port for serviceName -extern jlib_decl std::pair getExternalService(const char *serviceName); - -extern jlib_decl std::pair getDafileServiceFromConfig(const char *application); - extern jlib_decl char *getHPCCEnvVal(const char *name, const char *defaultValue); #endif