Skip to content

Get Node Pool Information for Remote Models#806

Merged
ryanweiler92 merged 1 commit intodevfrom
get-nodepools-info
May 12, 2025
Merged

Get Node Pool Information for Remote Models#806
ryanweiler92 merged 1 commit intodevfrom
get-nodepools-info

Conversation

@ryanweiler92
Copy link
Copy Markdown
Collaborator

@ryanweiler92 ryanweiler92 commented May 12, 2025

Description

Adds a reactor that fetches data about the deployed models in the node pools from the Kubernetes model scaler.

Changes Made

  • Adding GetNodePoolsInfoReactor
  • Adding a method to the KubernetesModelScaler class to fetch the data

How to Test

GetNodePoolsInfo()

Note: Requires KMS ingress env variable and admin mode.

Notes

Example return:

{"pools": [{"machine_type": "c3-standard-8", "models": [{"node": "gke-cfgai-standard-n-cpu-sapphire-poo-cb209854-33pn", "name": "dinov2-large", "namespace": "huggingface-models", "resources": {"memory_requests_gi": 8.01, "gpu_requests": 0, "cpu_requests": 2.01}, "url": "http://dinov2-large-predictor.huggingface-models.svc.cluster.local", "status": "True"}], "memory_total_gi": 32, "gpu_count": 0, "gpu_type": {}, "name": "cpu-sapphire-pool", "memory_available_gi": 18.84, "cpu_total": 8, "cpu_available": 5.47, "gpu_available": 0}, {"machine_type": "n1-standard-16", "models": [{"node": "gke-cfgai-standard-new-t4-gpu-pool-c1b8a6fd-03n2", "name": "florence-2-large", "namespace": "huggingface-models", "resources": {"memory_requests_gi": 10.01, "gpu_requests": 1, "cpu_requests": 1.01}, "url": "http://florence-2-large-predictor.huggingface-models.svc.cluster.local", "status": "True"}, {"node": "gke-cfgai-standard-new-t4-gpu-pool-c1b8a6fd-kkz6", "name": "stable-diffusion-v1-5", "namespace": "huggingface-models", "resources": {"memory_requests_gi": 10, "gpu_requests": 1, "cpu_requests": 1}, "url": "http://stable-diffusion-v1-5-predictor.huggingface-models.svc.cluster.local", "status": "True"}], "memory_total_gi": 240, "gpu_count": 1, "gpu_type": "nvidia.com/gpu", "name": "t4-gpu-pool", "memory_available_gi": 190.52, "cpu_total": 64, "cpu_available": 59.72, "gpu_available": 2}, {"machine_type": "g2-standard-12", "models": [], "memory_total_gi": 48, "gpu_count": 1, "gpu_type": "nvidia.com/gpu", "name": "l4-gpu-pool", "memory_available_gi": 41.58, "cpu_total": 12, "cpu_available": 11.46, "gpu_available": 1}], "message": "Successfully retrieved node pool information"}

@github-actions
Copy link
Copy Markdown

@CodiumAI-Agent /describe

@QodoAI-Agent
Copy link
Copy Markdown

Title

Get Node Pool Information for Remote Models


User description

Description

Adds a reactor that fetches data about the deployed models in the node pools from the Kubernetes model scaler.

Changes Made

  • Adding GetNodePoolsInfoReactor
  • Adding a method to the KubernetesModelScaler class to fetch the data

How to Test

GetNodePoolsInfo()

Note: Requires KMS ingress env variable and admin mode.

Notes


PR Type

Enhancement, Bug fix


Description

  • Enhance KubernetesModelScaler to fetch node pools info

  • Add jsonToMap helper for JSON-to-Map conversion

  • Introduce GetNodePoolsInfoReactor with admin check

  • Correct logger class in CanItRunReactor


Changes walkthrough 📝

Relevant files
Enhancement
KubernetesModelScaler.java
Support fetching node pools info                                                 

src/prerna/engine/impl/model/KubernetesModelScaler.java

  • Added getNodePoolsInfo HTTP GET method.
  • Parsed JSON response into Map structure.
  • Implemented jsonToMap recursive conversion.
  • Added error handling and logging.
  • +90/-0   
    GetNodePoolsInfoReactor.java
    Add node pools info reactor                                                           

    src/prerna/reactor/model/GetNodePoolsInfoReactor.java

  • Created reactor invoking getNodePoolsInfo.
  • Added admin permission check.
  • Returned node pools info as NounMetadata.
  • Logged errors on connection failure.
  • +36/-0   
    Bug fix
    CanItRunReactor.java
    Fix logger class reference                                                             

    src/prerna/reactor/model/CanItRunReactor.java

  • Updated logger to use CanItRunReactor.class.
  • Corrected class name in LogManager instantiation.
  • +1/-1     

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • @github-actions
    Copy link
    Copy Markdown

    @CodiumAI-Agent /review

    @github-actions
    Copy link
    Copy Markdown

    @CodiumAI-Agent /improve

    @QodoAI-Agent
    Copy link
    Copy Markdown

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Null Response Entity

    The code does not check if the HTTP response entity is null before calling EntityUtils.toString, which can cause a NullPointerException when the server returns no body.

    HttpEntity responseEntity = response.getEntity();
    String responseString = EntityUtils.toString(responseEntity);
    Error Logging

    In the catch block, the error log omits exception details; include the exception or stack trace in the logging call to facilitate debugging.

    } catch (Exception e) {
    	classLogger.error("Error connecting to the Kubernetes Model Scaler endpoint for Nodepool information..");
    	throw new RuntimeException("Failed to connect to Kubernetes Model Scaler endpoint: " + e.getMessage());
    Unauthorized Exception

    Throwing IllegalArgumentException for permission failures may confuse error handling; consider using a more specific exception or HTTP status for unauthorized access.

    if (!SecurityAdminUtils.userIsAdmin(this.insight.getUser())) {
    	throw new IllegalArgumentException("User does not have permission to query this endpoint.");
    }

    @ryanweiler92 ryanweiler92 merged commit c9e55cb into dev May 12, 2025
    4 checks passed
    @ryanweiler92 ryanweiler92 deleted the get-nodepools-info branch May 12, 2025 21:10
    @github-actions
    Copy link
    Copy Markdown

    @CodiumAI-Agent /update_changelog

    @QodoAI-Agent
    Copy link
    Copy Markdown

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Impact
    General
    Map JSON null to Java null

    Handle JSONObject.NULL values explicitly so they map to Java null instead of the
    JSON null placeholder. This ensures downstream code receives actual null values.

    src/prerna/engine/impl/model/KubernetesModelScaler.java [147-160]

     } else if (value instanceof JSONArray) {
         JSONArray array = (JSONArray) value;
         List<Object> list = new ArrayList<>();
         
         for (int i = 0; i < array.length(); i++) {
             Object item = array.get(i);
    -        if (item instanceof JSONObject) {
    +        if (item == JSONObject.NULL) {
    +            list.add(null);
    +        } else if (item instanceof JSONObject) {
                 list.add(jsonToMap((JSONObject) item));
             } else {
                 list.add(item);
             }
         }
         
         map.put(key, list);
    Suggestion importance[1-10]: 7

    __

    Why: Explicitly handling JSONObject.NULL ensures JSON nulls map to actual Java nulls, preventing downstream logic issues and improving data integrity.

    Medium
    Log exception with stack trace

    Include the caught exception in the log call to preserve the stack trace for
    debugging. Use the logger’s overloaded method that accepts a throwable.

    src/prerna/reactor/model/GetNodePoolsInfoReactor.java [31-34]

     } catch (Exception e) {
    -    classLogger.error("Error connecting to the Kubernetes Model Scaler endpoint for Nodepool information..");
    -    throw new RuntimeException("Failed to connect to Kubernetes Model Scaler endpoint: " + e.getMessage());
    +    classLogger.error("Error connecting to the Kubernetes Model Scaler endpoint for Nodepool information.", e);
    +    throw new RuntimeException("Failed to connect to Kubernetes Model Scaler endpoint: " + e.getMessage(), e);
     }
    Suggestion importance[1-10]: 7

    __

    Why: Including the caught exception in the classLogger.error call preserves the stack trace for debugging without altering control flow.

    Medium
    Add connection request timeout

    Add a connection request timeout to prevent hangs when the connection pool is
    exhausted. This will ensure the client fails fast under high load.

    src/prerna/engine/impl/model/KubernetesModelScaler.java [86-89]

     RequestConfig requestConfig = RequestConfig.custom()
             .setConnectTimeout(5000)
    +        .setConnectionRequestTimeout(5000)
             .setSocketTimeout(5000)
             .build();
    Suggestion importance[1-10]: 5

    __

    Why: Adding ConnectionRequestTimeout prevents hangs when the connection pool is exhausted, but it's a minor improvement for high-load scenarios.

    Low
    Possible issue
    Check for null HTTP response entity

    Add a null check for the response entity before calling EntityUtils.toString to
    avoid potential NullPointerExceptions when the response has no body. If the entity
    is null, throw or handle an appropriate exception.

    src/prerna/engine/impl/model/KubernetesModelScaler.java [100-101]

     HttpEntity responseEntity = response.getEntity();
    +if (responseEntity == null) {
    +    throw new RuntimeException("Empty response received from node pools endpoint");
    +}
     String responseString = EntityUtils.toString(responseEntity);
    Suggestion importance[1-10]: 6

    __

    Why: The suggestion correctly adds a null check before calling EntityUtils.toString() to avoid a potential NPE and improve robustness.

    Low

    manamittal added a commit that referenced this pull request May 20, 2025
    * fix(python): handle eval when it is a single line execution but there is string input with space (#756)
    
    * Update Dockerfile.tomcat (#757)
    
    * fix: tomcat builder setting env var
    
    * fix: updating tomcat to 9.0.104
    
    * Update Dockerfile.ubuntu22.04
    
    * Update Dockerfile.ubuntu22.04
    
    * Update Dockerfile.ubuntu22.04
    
    * feat: creating KubernetesModelScaler class (#763)
    
    * Update Dockerfile.ubuntu22.04
    
    * feat: adding ability to attach a file to a vector db source (#736)
    
    * Added AttachSourceToVectorDbReactor for uploading pdf file to an existing csv file and modified VectorFileDownloadReactor
    
    * fix: proper return for the download and matching the reactor name
    
    * fix: error for downloading single file vs multiple; error for copyToDirectory instead of copyFile
    
    * chore: renaming so reactor matches VectorFileDownload
    
    ---------
    
    Co-authored-by: Maher Khalil <themaherkhalil@gmail.com>
    
    * Update Dockerfile.ubuntu22.04
    
    * Update ubuntu2204.yml
    
    * Update ubuntu2204.yml
    
    * Update ubuntu2204_cuda.yml
    
    * Update Dockerfile.nvidia.cuda.12.5.1.ubuntu22.04
    
    * Update ubuntu2204_cuda.yml
    
    * Update ubuntu2204.yml
    
    * feat: exposing tools calling through models (#764)
    
    * 587 unit test for prernadsutil (#654)
    
    * test(unit): unit tests for the prerna.util.ds package
    
    * test(unit): unit tests for the prerna.util.ds.flatfile package
    
    * test(unit): removed reflections, added paraquet tests
    
    * test(unit): unit tests for the prerna.util.ds package
    
    * test(unit): unit tests for the prerna.util.ds.flatfile package
    
    * test(unit): removed reflections, added paraquet tests
    
    * Update ubuntu2204.yml
    
    * Update ubuntu2204.yml
    
    * Update ubuntu2204.yml
    
    * fix: update pipeline docker buildx version
    
    * fix: ignore buildx
    
    * fix: adjusting pipeline for cuda
    
    * feat: switching dynamic sas to default false (#766)
    
    * fix: changes to account for version 2.0.0 of pyjarowinkler (#769)
    
    * chore: using 'Py' instead of 'py' to be consistent (#770)
    
    * feat: full ast parsing of code to return evaluation of the last expression (#771)
    
    * Python Deterministic Token Trimming for Message Truncation (#765)
    
    * feat: deterministic-token-trimming
    
    * feat: modifying logic such that system prompt is second to last message for truncation
    
    ---------
    
    Co-authored-by: Maher Khalil <themaherkhalil@gmail.com>
    
    * fix: added date added column to enginepermission table (#768)
    
    * fix: add docker-in-docker container to run on sef-hosted runner (#773)
    
    Co-authored-by: Raul Esquivel <resmas.work@gmail.com>
    
    * fix: properly passing in the parameters from kwargs/smss into model limits calculation (#774)
    
    * fix: removing legacy param from arguments (#777)
    
    * fix: Fix docker cache build issue (#778)
    
    * adding no cache
    
    * adding no cache
    
    * feat: Adding Semantic Text Splitting & Token Text Splitting (#720)
    
    * [696] - build - Add chonky semantic text splitting - Added the function for chonky semantic text splitting and integrated with existing flow.
    
    * [696] - build - Add chonky semantic text splitting - Updated the code
    
    * [696] - build - Add chonky semantic text splitting - Updated the code comments
    
    * feat: adding reactor support through java
    
    * feat: updating pyproject.toml with chonky package
    
    * feat: check for default chunking method in smss
    
    * [696] - feat - Add chonku semantic text splitting - Resolved the conflicts
    
    * [696] - feat - Add chonky semantic text splitting - Organized the code.
    
    * feat: adding chunking by tokens and setting as default
    
    * updating comments on chunking strategies
    
    ---------
    
    Co-authored-by: Weiler, Ryan <ryanweiler92@gmail.com>
    Co-authored-by: kunal0137 <kunal0137@gmail.com>
    
    * feat: allowing for tools message in full prompt (#780)
    
    * UPDATE ::: Add docker in docker Dockerfiler (#784)
    
    * add docker in docker Dockerfile
    
    * Update Dockerfile.dind
    
    Remove python and tomcat arguments from Dockerfile
    
    * fix: remove-paddle-ocr (#786)
    
    * [#595] test(unit): adds unit test for prerna.engine.impl.model.kserve
    
    Co-authored-by: Ryan Weiler <ryanweiler92@gmail.com>
    
    * feat: Tag semoss image (#789)
    
    * adding changes for non-release docker build
    
    * adding non-release build logic to cuda-semoss builder
    
    * updating push branches
    
    * fix: branch names on docker builds
    
    * fix: branch names on docker builds cuda
    
    * fix: adding push condition - change to pyproject toml file; adding event input vars to env vars (#790)
    
    * fix: python builder toml file change (#792)
    
    * fix: Catch errors when calling pixels from Python (#787)
    
    Co-authored-by: Weiler, Ryan <ryanweiler92@gmail.com>
    
    * Creating db links between engines and default apps (#693)
    
    * create db links between engine and default app
    
    * Rename column APPID to TOOL_APP
    
    * feat: add database_tool_app to getUserEngineList
    
    ---------
    
    Co-authored-by: Weiler, Ryan <ryanweiler92@gmail.com>
    
    * Adding sort options to the myengines reactor (#479)
    
    * added sort feature to MyEnginesReactor and genericized reactor imports
    
    * formatting
    
    * overloading method
    
    * validate sortList
    
    ---------
    
    Co-authored-by: Ryan Weiler <ryanweiler92@gmail.com>
    
    * feat: cleaning up unused imports in MyEngine reactor (#793)
    
    * feat: Create Enum projectTemplate and update CreateAppFromTemplateReactor to accept existing appID for cloning applications (#621)
    
    Co-authored-by: kunal0137 <kunal0137@gmail.com>
    
    * Update GetEngineUsageReactor.java (#417)
    
    Co-authored-by: Maher Khalil <themaherkhalil@gmail.com>
    Co-authored-by: Ryan Weiler <ryanweiler92@gmail.com>
    
    * Issue 596: Adds Unit Tests for prerna/engine/impl/model/responses and workers (#727)
    
    * [#596] test(unit): adds unit tests
    
    * fix: implements ai-agents suggestions
    
    ---------
    
    Co-authored-by: Jeff Vitunac <jvitunac@gmail.com>
    Co-authored-by: Ryan Weiler <ryanweiler92@gmail.com>
    
    * 609 implement native blob storage for azure gcp and aws (#674)
    
    * Initial commit : implementation for azure blob storage
    
    * added dependency for azure in pom.xml
    
    * update logic to fetch the metadata from list details
    
    * changed functionality from listing containers to listing files within a selected container
    
    * initial commit for google cloud storage implementation
    
    * added field contant in enum class and removed unused method
    
    * add methods to parse comma-separated local and cloud paths
    
    * add methods to parse comma-separated local and cloud paths
    
    * implementation for aws s3 bucket
    
    * normalize container prefix path
    
    * merged all: implementation for azure, aws and gcp
    
    * refactor(storage): replace manual path normalization with normalizePath from Utility class
    
    ---------
    
    Co-authored-by: pvijayaraghavareddy <pvijayaraghavareddy@WORKSPA-6QV71G7.us.deloitte.com>
    Co-authored-by: Parth <parthpatel3@deloitte.com>
    Co-authored-by: Ryan Weiler <ryanweiler92@gmail.com>
    
    * Get Node Pool Information for Remote Models (#806)
    
    * 590 unit test for prernaengineimpl (#808)
    
    * test(unit): update to filesystems hijacking for testing files
    
    * test: start of unit tests for abstract database engine
    
    * test(unit): added unit test for prerna.engine.impl
    
    * test(unit): finsihed tests for prerna.engine.impl
    
    * test(unit): adding back unused assignment
    
    ---------
    
    Co-authored-by: Ryan Weiler <ryanweiler92@gmail.com>
    
    * Creating WordCountTokenizer Class (#802)
    
    * feat: creating word count tokenizer class && falling back to word count tokenizer if tiktok fails
    
    * feat: updating comment
    
    * feat: setting default chunking method as recursive (#810)
    
    * Unit tests fixes and Unit test Class file location updates (#812)
    
    * test(unit): moved tests to correct packages
    
    * test(unit): fixed a couple of unit tests
    
    * VectorDatabaseQueryReactor: output divider value for word doc chunks always 1 (#804)
    
    * Code implementation for #733
    
    * feat: Added code to resolve Divider page issue
    
    * Console output replaced by LOGGERs as per review comments
    
    * feat: replaced Console with Loggers
    
    ---------
    
    Co-authored-by: Varaham <katchabi50@gmail.com>
    Co-authored-by: Ryan Weiler <ryanweiler92@gmail.com>
    
    * GetCurrentUserReactor (#818)
    
    Adding GetCurrentUserReactor to return user info including if user is an admin.
    
    * Python User Class (#819)
    
    * fix: trimming properties read from smss; fix: logging commands before executing (#821)
    
    * Updating getNodePoolsInfo() to parse and return zk info and models active actual (#822)
    
    * feat: update get node pool information for zk info and models active actual
    
    * feat: get remote model configs
    
    * Add unit tests for package prerna\engine\impl\vector (#728)
    
    * Create ChromaVectorDatabaseEngineUnitTests.java
    
    * completed tests for ChromaVectorDatabaseEngine class
    
    * [#604] test(unit): Created ChromaVectorDatabaseEngine unit tests
    
    * [604] tests(unit) : Completed test cases for ChromaVectorDatabaseEngine; update File operations to nio operations in ChromaVectorDatabaseEngine.java
    
    * [#604] tests(unit): added unit tests for all vector database engines and util classes in the prerna\engine\impl\vector package
    
    * [604] test(unit): replaced creating file paths with string literals with java.nio Paths.resolve/Paths.get methods
    
    ---------
    
    Co-authored-by: Maher Khalil <themaherkhalil@gmail.com>
    Co-authored-by: Ryan Weiler <ryanweiler92@gmail.com>
    
    * feat: adding to the return of getenginemetadata (#813)
    
    * feat: adding to the return of getenginemetadata
    
    * fix: removing throws
    
    ---------
    
    Co-authored-by: Arash Afghahi <48933336+AAfghahi@users.noreply.github.com>
    Co-authored-by: Ryan Weiler <ryanweiler92@gmail.com>
    
    * 718 create a single reactor to search both engines and apps (#794)
    
    * feat(engineProject): Initial commit
    
    * chore: 718 create a single reactor to search both engines and apps
    
    * chore: 718 create a single reactor to search both engines and apps
    
    ---------
    
    Co-authored-by: Ryan Weiler <ryanweiler92@gmail.com>
    Co-authored-by: Vijayaraghavareddy <pvijayaraghavareddy@deloitte.com>
    
    * feat: update openai wrapper to handle multiple images (#832)
    
    * feat: adding user room map (#840)
    
    * feat: hiding side menu bar for non admins (#833)
    
    * Side menu changes
    
    * Review Comments fixed
    
    * Flag is renamed in  Constants.java
    
    * Review Comment fixed in Utility.java
    
    * fix: cleaning up defaults and comments
    
    ---------
    
    Co-authored-by: kunal0137 <kunal0137@gmail.com>
    
    ---------
    
    Co-authored-by: Maher Khalil <themaherkhalil@gmail.com>
    Co-authored-by: kunal0137 <kunal0137@gmail.com>
    Co-authored-by: Ryan Weiler <ryanweiler92@gmail.com>
    Co-authored-by: ManjariYadav2310 <manjayadav@deloitte.com>
    Co-authored-by: dpartika <dpartika@deloitte.com>
    Co-authored-by: Raul Esquivel <resmas.work@gmail.com>
    Co-authored-by: Pasupathi Muniyappan <pasupathi.muniyappan@kanini.com>
    Co-authored-by: resmas-tx <131498457+resmas-tx@users.noreply.github.com>
    Co-authored-by: AndrewRodddd <62724891+AndrewRodddd@users.noreply.github.com>
    Co-authored-by: radkalyan <107957324+radkalyan@users.noreply.github.com>
    Co-authored-by: samarthKharote <samarth.kharote@kanini.com>
    Co-authored-by: Shubham Mahure <shubham.mahure@kanini.com>
    Co-authored-by: rithvik-doshi <81876806+rithvik-doshi@users.noreply.github.com>
    Co-authored-by: Mogillapalli Manoj kumar <86736340+Khumar23@users.noreply.github.com>
    Co-authored-by: Jeff Vitunac <jvitunac@gmail.com>
    Co-authored-by: pvijayaraghavareddy <pvijayaraghavareddy@WORKSPA-6QV71G7.us.deloitte.com>
    Co-authored-by: Parth <parthpatel3@deloitte.com>
    Co-authored-by: KT Space <119169984+Varaham@users.noreply.github.com>
    Co-authored-by: Varaham <katchabi50@gmail.com>
    Co-authored-by: ericgonzal8 <ericgonzalez8@deloitte.com>
    Co-authored-by: Arash Afghahi <48933336+AAfghahi@users.noreply.github.com>
    Co-authored-by: Vijayaraghavareddy <pvijayaraghavareddy@deloitte.com>
    Co-authored-by: ammb-123 <ammb@deloitte.com>
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    2 participants