add the feature for sorting agent table and searching by name ! by Rupam-It · Pull Request #722 · antrea-io/antrea-ui

Rupam-It · 2025-04-22T15:59:23Z

#310
added feature

pagination 10 agent
you can sort by one column at a time
search feature by name is added

Signed-off-by: Rupam-It <mannarupam3@gmail.com>

antoninbas

some initial comments

antoninbas · 2025-04-26T00:16:47Z

aren't the logos backwards?
this one should be descending because it goes from larger at the top to smaller at the bottom

antoninbas · 2025-04-26T00:17:05Z

+            </g>
+        </svg>
+    );
+} 


add missing newline character

antoninbas · 2025-04-26T00:18:45Z


 function ComponentSummary<T>(props: {title: string, data: T[], propertyNames: Property[], getProperties: (x: T) => string[]}) {
+    const itemsPerPage = 10; // Display 10 items per page for Agents table
+    const isAgentTable = props.title === 'Agents';


rather than this, could we have a "sortable" property?

antoninbas · 2025-04-26T00:19:55Z

    );
 }

+function generateFakeAgents(n: number): AgentInfo[] {


you should add unit tests in summary.test.tsx for the new sorting feature. The tests can use the synthetic data that you have here.

as a matter of fact, I think the test file already has some helper functions potentially to help with synthetic data generation?

antoninbas · 2025-04-26T00:27:18Z

+            // If in development mode, generate fake agent data for UI testing
+            if (import.meta.env.DEV) {


Even in development mode, one usually connects to an actual K8s cluster and wants real agent data?

I understand that this is convenient, as usually the test clusters used for development are small and don't have a large number of Nodes / agents. So I think we can keep it, but 1) the generation of the synthetic data should be in a separate file if possible for readability, 2) this should be driven by a variable that can be set in .env.development / .env.development.local. You could set the variable in .env.development for reference, but it should be commented out by default. I don't know if there is a better / more standard way of doing it.

antoninbas · 2025-04-26T00:30:34Z

+    const isAgentTable = props.title === 'Agents';
+
+    const [currentPage, setCurrentPage] = useState(1);
+    const [sortConfig, setSortConfig] = useState<SortConfig | null>(null);


assuming my understanding of the code is correct, by default there is no sorting (which is the current behavior)?

@Rupam-It please confirm

Signed-off-by: Rupam-It <mannarupam3@gmail.com>

Rupam-It · 2025-05-03T17:35:33Z

Hey @antoninbas ,

Added sorting, search, and pagination to the Agents table for better usability with large datasets.
Made ComponentSummary generic to support configurable table features across components.
Moved generateFakeAgents to utils/fakeData.ts; added VITE_USE_FAKE_AGENTS to control mock data in dev.
Improved UI with modern chevron sort icons, proper sort directions, and visual feedback.
Disabled pagination buttons at boundaries for better UX.
Added unit tests for sorting (all data types), search, and pagination to ensure reliability.

Signed-off-by: Rupam-It <mannarupam3@gmail.com>

Rupam-It · 2025-05-03T18:24:22Z

I also have test in the web page !
all working fine

searching by name
sorting by column
pagination (default 10 , i have test it with 2 it working)

antoninbas · 2025-05-07T20:23:04Z

 VITE_API_SERVER=http://localhost:8080
+
+# Uncomment the line below to enable fakeData(agent name) in development mode
+VITE_USE_SYNTHETIC_DATA=true


remember to comment the line out for the code you check in

antoninbas · 2025-05-07T20:23:29Z

@@ -1 +1,4 @@
 VITE_API_SERVER=http://localhost:8080
+
+# Uncomment the line below to enable fakeData(agent name) in development mode


Suggested change

# Uncomment the line below to enable fakeData(agent name) in development mode

# Uncomment the line below to enable synthetic Agent data in development mode.

antoninbas · 2025-05-07T20:24:22Z

+export function generateFakeAgents(n: number): AgentInfo[] {
+    // Helper function to generate random agent names
+    const generateAgentName = () => {
+        const prefixes = ['antrea', 'k8s', 'node', 'worker', 'master'];


Please avoid using "master", and prefer more inclusive terminology

antoninbas · 2025-05-07T20:25:41Z

+        const prefixes = ['antrea', 'k8s', 'node', 'worker', 'master'];
+        const suffixes = ['-agent', '-node', '-vm', '-host', ''];
+        const prefix = prefixes[Math.floor(Math.random() * prefixes.length)];
+        const suffix = suffixes[Math.floor(Math.random() * suffixes.length)];
+        const id = Math.random() < 0.3 ? 
+            Math.random().toString(36).substring(2, 6) : // alphanumeric
+            Math.floor(Math.random() * 1000).toString(); // numeric
+        return `${prefix}${suffix}-${id}`;


I don't understand the value of having such "complicated" code for generating agent names?
why not simply use k8s-node-control-plane-1, k8s-node-worker-1, k8s-node-worker-2, ...?

antoninbas · 2025-05-07T20:27:01Z

+        const major = Math.floor(Math.random() * 3);
+        const minor = Math.floor(Math.random() * 10);
+        const patch = Math.floor(Math.random() * 20);
+        return `v${major}.${minor}.${patch}`;


here again, I see little value in having totally random version numbers
I would actually suggest having 2 possible different version numbers (e.g., 2.2.3 and 2.3.0), which would be representative of what you could have when you are in the middle of updating Antrea in a large cluster

antoninbas · 2025-05-07T20:27:48Z

+        return `v${major}.${minor}.${patch}`;
+    };
+
+    // Helper function to generate random timestamps within last 30 days


probably much too large of a window when it comes to the heartbeat timestamp, maybe use 5 minutes instead of 30 days?

antoninbas · 2025-05-07T20:28:47Z

+    const generateSubnets = () => {
+        const count = Math.floor(Math.random() * 3) + 1; // 1-3 subnets
+        const subnets = [];
+        for (let i = 0; i < count; i++) {
+            const ipv4 = `${Math.floor(Math.random() * 255)}.${Math.floor(Math.random() * 255)}.${Math.floor(Math.random() * 255)}.0`;
+            const ipv6 = `fd${Math.random().toString(16).substr(2, 2)}:${Math.random().toString(16).substr(2, 4)}::`;
+            subnets.push(Math.random() > 0.5 ? `${ipv4}/24` : `${ipv6}/64`);
+        }
+        return subnets;


here as well, please use something more representative than a random subnet, you can use consecutive subnets

antoninbas · 2025-05-07T20:28:57Z

+    };
+
+    // Helper function to generate OVS version
+    const generateOVSVersion = () => {


same comment as above (for the Antrea version)

antoninbas · 2025-05-07T20:29:44Z

+    const isAgentTable = props.title === 'Agents';
+
+    const [currentPage, setCurrentPage] = useState(1);
+    const [sortConfig, setSortConfig] = useState<SortConfig | null>(null);


@Rupam-It please confirm

antoninbas · 2025-05-07T20:35:11Z

+            const aValue = getProperties(a)[sortKeyIndex];
+            const bValue = getProperties(b)[sortKeyIndex];
+
+            // Custom numerical sort for 'Name' column (agent-N)


this is not good, once you start doing that the ComponentSummary is not generic at all anymore

either you have to use default string sorting (lexicographic) for all properties, or you need to update getProperties so that it's not limited to returning an array of strings. Maybe it could return (string | number | someType)[] and then the sorting logic would be different based on the type?

if we sort every thing as a string then
after node-1 come node-10 not node-2!
as i have already implemented that string+num sorting !
so in our logic node-2 will comes after node-1 as it is more acceptable to all !
what's your thought?

Rupam-It · 2025-05-08T15:09:43Z

#722 (comment)
hey @antoninbas i have make the desire change i
1. Refined fakeData.ts

Replaced "master" terminology with more inclusive "control-plane"
Created structured, predictable agent naming scheme (k8s-node-control-plane-1, k8s-node-worker-N)
Limited version numbers to two specific versions (v2.2.3 and v2.3.0) to simulate clusters during upgrades
Reduced heartbeat timestamp window from 30 days to 5 minutes for more realistic data
Implemented sequential, representative subnet patterns instead of random ones
Added option for randomized agent order to properly test sorting functionality

Made node names match agent names for consistency

2. Made ComponentSummary Truly Generic

Redesigned the component to use proper type-based sorting rather than special cases
Implemented PropertyValuePair approach that separates display values from sort values
Added support for multiple data types (string, number, Date, tuple) with appropriate sorting logic
Created a hierarchical sorting system for node names that maintains proper ordering:

Control-plane nodes appear before worker nodes
Node numbers sort properly (1, 2, 3... 10, 11) instead of lexicographically
3. Enhanced Code Maintainability

Removed hard-coded column-specific sorting logic
Made the component follow the "Open/Closed Principle" - extendable without modifying core code
Improved comments throughout for better code comprehension
Maintained all the original functionality while making the code more robust

see this video for more clearness!
Screencast from 2025-05-08 20-35-13.webm

…akeData, added type-based sorting and better code maintainability. Signed-off-by: Rupam-It <mannarupam3@gmail.com>

Signed-off-by: Rupam-It <mannarupam3@gmail.com>

antoninbas · 2025-05-08T20:58:06Z

+        const generateOVSVersion = () => {
+            // Only two possible OVS versions to simulate a cluster during upgrade
+            return actualIndex % 4 === 0 ? '3.0.0' : '2.17.3';
+        };


you should generate the Antrea version and OVS version at the same time, because there should be a 1-1 mapping between them

antoninbas · 2025-05-08T21:00:08Z

+            // Generate sequential IPv4 subnets
+            if (count > 0) {
+                // Base subnet with sequential third octet
+                subnets.push(`10.10.${actualIndex % 255}.0/24`);
+            }
+
+            // Add IPv6 subnet for some nodes
+            if (count > 1) {
+                // Sequential IPv6 subnets
+                subnets.push(`fd00:10:10:${actualIndex % 100}::/64`);
+            }


All Nodes should have "consistent" subnets (e.g., a single IPv4 subnet, or one IPv4 subnet + one IPv6 subnet).
I don't think it's worth having an error case here (Node missing subnet)

antoninbas · 2025-05-08T21:01:16Z

+            // Generate sequential IPv4 subnets
+            if (count > 0) {
+                // Base subnet with sequential third octet
+                subnets.push(`10.10.${actualIndex % 255}.0/24`);


If you have too many fake agents, you will have duplicate subnets. Should there be an assertion to limit the number of agents?

antoninbas · 2025-05-08T21:01:27Z

+
+# Uncomment the line below to enable synthetic Agent data in development mode.
+# VITE_USE_SYNTHETIC_DATA=true
+


remove empty line

antoninbas · 2025-05-08T21:02:43Z

+// Helper function to extract node type and numeric suffix for proper sorting
+function getNodeNameSortValue(name: string): [string, number] {
+    // Match patterns like "k8s-node-worker-123" or "k8s-node-control-plane-1"
+    const match = name.match(/^(.*?)(control-plane|worker)-(\d+)$/);
+    if (match) {
+        // Extract the node type and number
+        const prefix = match[1];
+        const nodeType = match[2]; // 'control-plane' or 'worker'
+        const numericPart = parseInt(match[3], 10);
+
+        // Return as tuple with nodeType first (for primary sorting) and number second
+        // This ensures control-plane comes before worker when sorting
+        return [nodeType, numericPart];
+    }
+    // Fall back to a default value if pattern doesn't match
+    return ['unknown', 0];


why even have a specific pattern here? this won't match most clusters
the code should be generic

antoninbas · 2025-05-08T21:05:27Z

+        { display: agent.metadata.name, sortValue: getNodeNameSortValue(agent.metadata.name) },
+        { display: agent?.version ?? 'Unknown', sortValue: agent?.version ?? 'Unknown' },
+        { display: refToString(agent.podRef), sortValue: refToString(agent.podRef) },
+        { display: refToString(agent.nodeRef), sortValue: refToString(agent.nodeRef) },
+        { display: localPodNum.toString(), sortValue: localPodNum },
+        { display: agent.nodeSubnets?.join(',') ?? 'None', sortValue: agent.nodeSubnets?.join(',') ?? 'None' },
+        { display: agent?.ovsInfo?.version ?? 'Unknown', sortValue: agent?.ovsInfo?.version ?? 'Unknown' },
+        { display: healthy, sortValue: healthy },
+        { display: lastHeartbeat, sortValue: new Date(lastHeartbeat) },


You didn't reply to my original comment (#722 (comment)), so I don't know your thought process, but I don't think this is better than sorting purely based on the value type

so , i have write the sorting logic more refine you can take a look now!

…e the hard coded name form the agent name Signed-off-by: Rupam-It <mannarupam3@gmail.com>

Rupam-It force-pushed the issue-310 branch from 7683a9a to fe4964e Compare April 22, 2025 16:18

Rupam-It added 2 commits April 22, 2025 21:52

pagination support added

e9303e4

Signed-off-by: Rupam-It <mannarupam3@gmail.com>

changes made on the demand for antrea-io#310 (comment)

fa7daef

Signed-off-by: Rupam-It <mannarupam3@gmail.com>

Rupam-It force-pushed the issue-310 branch from fe4964e to fa7daef Compare April 22, 2025 16:22

antoninbas reviewed Apr 26, 2025

View reviewed changes

further improvement

80dbb33

Signed-off-by: Rupam-It <mannarupam3@gmail.com>

Rupam-It added 2 commits May 3, 2025 23:07

old icon deleted

452eb19

Signed-off-by: Rupam-It <mannarupam3@gmail.com>

new line added

dcbd335

Signed-off-by: Rupam-It <mannarupam3@gmail.com>

antoninbas reviewed May 7, 2025

View reviewed changes

Rupam-It added 2 commits May 8, 2025 20:41

Refactored ComponentSummary to be generic, improved agent naming in f…

7e8caf4

…akeData, added type-based sorting and better code maintainability. Signed-off-by: Rupam-It <mannarupam3@gmail.com>

extra line added

3289d1b

Signed-off-by: Rupam-It <mannarupam3@gmail.com>

antoninbas reviewed May 8, 2025

View reviewed changes

more refine logic has done for synthetic data creation and also remov…

d255cbc

…e the hard coded name form the agent name Signed-off-by: Rupam-It <mannarupam3@gmail.com>

+                          </g>
+                      </svg>
+                  );
+              } 

                
                    No newline at end of file

		// If in development mode, generate fake agent data for UI testing
		if (import.meta.env.DEV) {

		@@ -1 +1,4 @@
		VITE_API_SERVER=http://localhost:8080

		# Uncomment the line below to enable fakeData(agent name) in development mode

	# Uncomment the line below to enable fakeData(agent name) in development mode
	# Uncomment the line below to enable synthetic Agent data in development mode.


		# Uncomment the line below to enable synthetic Agent data in development mode.
		# VITE_USE_SYNTHETIC_DATA=true

Uh oh!

Conversation

Rupam-It commented Apr 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

antoninbas left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Rupam-It commented May 3, 2025

Uh oh!

Rupam-It commented May 3, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Rupam-It commented May 8, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Rupam-It commented Apr 22, 2025 •

edited

Loading