Skip to content
Open
Show file tree
Hide file tree
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
14 changes: 14 additions & 0 deletions web/cypress/fixtures/flowcollector/fc_lokiWithoutLokiStack.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
kind: FlowCollector
apiVersion: flows.netobserv.io/v1beta2
metadata:
name: cluster
spec:
agent:
ebpf:
sampling: 1
type: eBPF
loki:
enable: true
mode: LokiStack
lokiStack:
name: loki
62 changes: 62 additions & 0 deletions web/cypress/integration-tests/flowcollector_status.cy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import { Operator } from "@views/netobserv"
import { flowcollectorStatusPage, flowcollectorStatusSelectors } from "@views/flowcollector-status"

describe('Network_Observability FlowCollector status error scenario', { tags: ['Network_Observability'] }, function () {

before('setup', function () {
cy.adminCLI(`oc adm policy add-cluster-role-to-user cluster-admin ${Cypress.env('LOGIN_USERNAME')}`)
cy.uiLogin(Cypress.env('LOGIN_IDP'), Cypress.env('LOGIN_USERNAME'), Cypress.env('LOGIN_PASSWORD'))

Operator.install()
cy.checkStorageClass(this)

Operator.createFlowcollector("LokiWithoutLokiStack")
})
Comment on lines +6 to +14
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Error-scenario before hook skips the "FlowCollector already exists" guard.

Unlike the success scenario which uses Operator.createFlowcollector() (which deletes any pre-existing FC and waits for console refresh), this block oc applys directly. If the previous describe's after failed for any reason (see the outstanding concern about after hooks not running when before throws), the old Ready FlowCollector will be patched in-place rather than replaced, and the 120s wait at Line 127 may observe stale Ready=True status before flipping. Suggest calling Operator.deleteFlowCollector() first, or reusing Operator.createFlowcollector('LokiWithoutStack') (after fixing the Ready assertion flagged in netobserv.ts), plus a console-refresh wait similar to createFlowcollector.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/cypress/integration-tests/flowcollector_status.cy.ts` around lines 113 -
122, The before hook uses cy.deployFlowcollectorFromFixture(...) which applies a
FlowCollector in-place and can leave a previously Ready FC intact; update this
setup to either call Operator.deleteFlowCollector() first to remove any existing
FlowCollector before applying the fixture, or reuse
Operator.createFlowcollector('LokiWithoutStack') (after addressing the Ready
assertion in netobserv.ts) so it deletes, creates and waits for console refresh
like the success path; ensure the chosen change adds the same console-refresh
wait used by Operator.createFlowcollector to avoid observing stale Ready=True
state.


it("(OCP-88744, kapjain, Network_Observability) Verify error status when Loki enabled without LokiStack", function () {
// Visit status page and wait for Ready condition to show False (error state)
flowcollectorStatusPage.visit()
cy.get(flowcollectorStatusSelectors.readyRow, { timeout: 120000 }).should('exist')
.should('have.attr', 'data-test-status', 'False')
cy.get(flowcollectorStatusSelectors.readyRow)
.should('have.attr', 'data-test-reason')
.and('not.equal', 'Pending')
.and('not.equal', 'Valid')

// Verify WaitingFLPMonolith condition shows error about loki-gateway-ca-bundle
cy.get(flowcollectorStatusSelectors.flpMonolithRow).should('exist')
.should('have.attr', 'data-test-status', 'True')
cy.get(flowcollectorStatusSelectors.flpMonolithRow).parent()
.should('contain.text', 'loki-gateway-ca-bundle')

// Verify Flowlogs Pipeline component shows error about loki-gateway-ca-bundle
cy.contains('td', 'Flowlogs Pipeline').parent('tr')
.should('contain.text', 'loki-gateway-ca-bundle')
Comment on lines +33 to +34
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fragile row lookup — anchor to the component table.

cy.contains('td', 'Flowlogs Pipeline').parent('tr') matches the first <td> containing that text anywhere on the page. If the string also appears in a Details/message column elsewhere (common in error states), the test silently asserts against the wrong row. Anchor the lookup to the component statuses table, or add a stable data-test/id on the row in resource-status.tsx.

♻️ Suggested tightening
-        cy.contains('td', 'Flowlogs Pipeline').parent('tr')
-            .should('contain.text', 'loki-gateway-ca-bundle')
+        cy.contains('Component statuses')
+            .parents('section, [role="region"], .pf-v6-c-card, .pf-c-card').first()
+            .contains('td', /^Flowlogs Pipeline$/).parent('tr')
+            .should('contain.text', 'loki-gateway-ca-bundle')

As per coding guidelines, web/cypress/**/*.ts: Verify E2E test stability, proper waits, and selector resilience.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
cy.contains('td', 'Flowlogs Pipeline').parent('tr')
.should('contain.text', 'loki-gateway-ca-bundle')
cy.contains('Component statuses')
.parents('section, [role="region"], .pf-v6-c-card, .pf-c-card').first()
.contains('td', /^Flowlogs Pipeline$/).parent('tr')
.should('contain.text', 'loki-gateway-ca-bundle')
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/cypress/integration-tests/flowcollector_status.cy.ts` around lines 139 -
140, The test in flowcollector_status.cy.ts uses a global contains lookup for
the cell text "Flowlogs Pipeline" which can match elsewhere; scope the selector
to the component statuses table or add a stable test id on the row in
resource-status.tsx so the spec targets the correct row. Concretely, either
update the E2E test to find the table element for component statuses first and
then search for the td containing "Flowlogs Pipeline" within that table, or add
a deterministic data-test/data-id on the row rendered by resource-status.tsx
(e.g. per-component row id keyed by component name) and change the assertion in
flowcollector_status.cy.ts to select that data-test/id before asserting the
presence of "loki-gateway-ca-bundle".


// Verify WaitingLokiStack condition shows LokiStack not found error
cy.get(flowcollectorStatusSelectors.lokiStackRow).should('exist')
.should('have.attr', 'data-test-status', 'True')
cy.get(flowcollectorStatusSelectors.lokiStackRow)
.should('contain.text', 'Loki is configured in LokiStack mode, but LokiStack API is missing')
Comment on lines +36 to +40
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Do not assert the environment-specific LokiStack API message.

This exact text only holds when the LokiStack API is absent. On clusters where the CRD exists but the loki stack is missing, the condition can still be valid while the message differs. Prefer the stable reason from the operator and only loosely assert the row context.

Proposed adjustment
         // Verify WaitingLokiStack condition shows LokiStack not found error
         cy.get(flowcollectorStatusSelectors.lokiStackRow).should('exist')
             .should('have.attr', 'data-test-status', 'True')
+            .should('have.attr', 'data-test-reason', 'CantFetchLokiStack')
         cy.get(flowcollectorStatusSelectors.lokiStackRow)
-            .should('contain.text', 'Loki is configured in LokiStack mode, but LokiStack API is missing')
+            .should('contain.text', 'LokiStack')

As per coding guidelines, web/cypress/**/*.ts: Verify E2E test stability, proper waits, and selector resilience.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/cypress/integration-tests/flowcollector_status.cy.ts` around lines 143 -
147, Replace the brittle exact-message assertion on
flowcollectorStatusSelectors.lokiStackRow with a stable check: keep the
existence and data-test-status='True' assertions, remove the full
environment-specific text match, and instead assert a stable operator-provided
reason (e.g., the data-test-reason attribute or a shorter contains like
'Loki'/'LokiStack') and ensure the test waits for the row to appear (reuse the
existing cy.get(...).should('exist') pattern) so the assertion is resilient
across clusters where the CRD exists but the stack is missing.


// Verify WaitingFLPParent condition shows FLP error
cy.get(flowcollectorStatusSelectors.flpParentRow).should('exist')
.should('have.attr', 'data-test-status', 'True')
.and('have.attr', 'data-test-reason', 'FLPError')

// Verify status icon tooltip shows error
cy.get(flowcollectorStatusSelectors.statusButton)
.find('span span').trigger('mouseenter', { force: true })
cy.get(flowcollectorStatusSelectors.statusTooltip, { timeout: 10000 })
.should('contain.text', 'FlowCollector has errors')

// Verify "Open Network Traffic page" button is disabled
cy.byLegacyTestID('open-network-traffic').should('exist')
.should('have.attr', 'aria-disabled', 'true')
})

after("all tests", function () {
Operator.deleteFlowCollector()
cy.adminCLI(`oc adm policy remove-cluster-role-from-user cluster-admin ${Cypress.env('LOGIN_USERNAME')}`)
})
})
97 changes: 87 additions & 10 deletions web/cypress/integration-tests/static_plugin.cy.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import { netflowPage, overviewSelectors, pluginSelectors } from "@views/netflow-page"
import { Operator } from "@views/netobserv"
import {flowcollectorStatusPage, flowcollectorStatusSelectors} from "@views/flowcollector-status";
import {searchPage} from "@views/search";

describe('(OCP-84156 Network_Observability) StaticPlugin test', { tags: ['Network_Observability'] }, function () {
describe('(OCP-84156 OCP-88744 Network_Observability) StaticPlugin test with Status Check', { tags: ['Network_Observability'] }, function () {

before('any test', function () {
cy.adminCLI(`oc adm policy add-cluster-role-to-user cluster-admin ${Cypress.env('LOGIN_USERNAME')}`)
Expand All @@ -12,19 +14,56 @@ describe('(OCP-84156 Network_Observability) StaticPlugin test', { tags: ['Networ
Operator.createFlowcollector("StaticPlugin")
})

it("(OCP-84156, aramesha, Network_Observability) Edit flowcollector form view", function () {
it("(OCP-84156, OCP-88744 aramesha, Network_Observability) Edit flowcollector form view with Status Check", function () {
// Edit flowcollector form view to update sampling to 1
cy.visit('k8s/cluster/flows.netobserv.io~v1beta2~FlowCollector/status')
flowcollectorStatusPage.visit()
// Verify status page title with status icon and tooltip on hover
cy.contains('Network Observability FlowCollector status').should('exist')
cy.get(flowcollectorStatusSelectors.statusButton).should('exist')
.find('span span').trigger('mouseenter', { force: true })
cy.get(flowcollectorStatusSelectors.statusTooltip, { timeout: 10000 })
.should('contain.text', 'FlowCollector is ready')

// Verify component statuses table headers
cy.contains('Component statuses').should('exist')
cy.contains('th', 'Component').should('exist')
cy.contains('th', 'State').should('exist')
cy.contains('th', 'Replicas').should('exist')
cy.contains('th', 'Details').should('exist')

// Verify component rows
cy.contains('eBPF Agent').should('exist')
cy.contains('Flowlogs Pipeline').should('exist')
cy.contains('Console Plugin').should('exist')
cy.contains('Loki').should('exist')
cy.contains('Monitoring').should('exist')

// Verify "Open Network Traffic page" button is enabled when FC is ready
cy.byLegacyTestID('open-network-traffic').should('exist')
.should('not.have.attr', 'aria-disabled', 'true')

// Verify demoloki install warning alert at top of status page
cy.get(flowcollectorStatusSelectors.configIssueRow).should('exist')
.should('have.attr', 'data-test-status', 'True')
.should('have.attr', 'data-test-reason', 'Warnings')
cy.contains('Configuration warnings').should('exist')

// Verify Conditions
cy.contains('Conditions').should('exist')
cy.get(flowcollectorStatusSelectors.readyRow)
.should('have.attr', 'data-test-status', 'True')
cy.get(flowcollectorStatusSelectors.agentReadyRow).should('exist')
cy.get(flowcollectorStatusSelectors.pluginReadyRow).should('exist')
cy.get(flowcollectorStatusSelectors.monitoringReadyRow).should('exist')
cy.get(pluginSelectors.editFlowcollector).click()
cy.get('#root_spec_agent_accordion-toggle').click()
cy.get('#root_spec_agent_ebpf_sampling').clear().type('1')
cy.get(pluginSelectors.update).click()
// Wait for flowcollector to get ready
cy.wait(20000)
cy.get('[id=Ready-row]', { timeout: 60000 }).each($td => {
cy.wrap($td).should('have.attr', 'data-test-status', 'True')
cy.wrap($td).should('have.attr', "data-test-reason", 'Ready')
})
cy.get(flowcollectorStatusSelectors.readyRow).should('exist')
.should('have.attr', 'data-test-status', 'True')
.should('have.attr', 'data-test-reason', 'Ready')

cy.get(pluginSelectors.openNetworkTraffic).click()
// Verify PacketDrop data is seen
Expand All @@ -35,12 +74,50 @@ describe('(OCP-84156 Network_Observability) StaticPlugin test', { tags: ['Networ
cy.checkPanelsNum(6);

cy.checkNetflowTraffic()
})

afterEach("test", function () {
netflowPage.resetClearFilters()
})

it("(OCP-88744, kapjain, Network_Observability) Verify status indicator on Network Health page", function () {
cy.visit('/network-health')
// cy.get('#content-scrollable', { timeout: 30000 }).should('exist')
cy.get(flowcollectorStatusSelectors.statusIndicator).should('exist')
.find('span span').trigger('mouseenter', { force: true })
cy.get(flowcollectorStatusSelectors.statusTooltip, { timeout: 10000 })
.should('contain.text', 'FlowCollector is ready')

cy.get(flowcollectorStatusSelectors.statusIndicator).click()
cy.contains('Network Observability FlowCollector status', { timeout: 30000 }).should('exist')
})

it("(OCP-88744, kapjain, Network_Observability) Verify status indicator on Network Traffic page", function () {
cy.visit('/netflow-traffic')
// cy.get('#overview-container', { timeout: 60000 }).should('exist')
cy.get(flowcollectorStatusSelectors.statusIndicator).should('exist')
.find('span span').trigger('mouseenter', { force: true })
cy.get(flowcollectorStatusSelectors.statusTooltip, { timeout: 10000 })
.should('contain.text', 'FlowCollector is ready')

cy.get(flowcollectorStatusSelectors.statusIndicator).click()
cy.contains('Network Observability FlowCollector status', { timeout: 30000 }).should('exist')
})

it("(OCP-88744, kapjain, Network_Observability) Verify FlowCollector status via search and cluster columns", function () {
// Search for FlowCollector via search page
searchPage.navToSearchPage()
searchPage.chooseResourceType('FlowCollector')
cy.byTestID('data-view-table', { timeout: 30000 }).should('exist')
cy.byTestID('data-view-cell-cluster-name').should('exist')

// Verify additionalPrinterColumn headers
cy.byTestID('additional-printer-column-header-Agent').should('exist')
cy.byTestID('additional-printer-column-header-Processor').should('exist')
cy.byTestID('additional-printer-column-header-Plugin').should('exist')
cy.byTestID('additional-printer-column-header-Status').should('exist')

// Verify status column shows Ready
cy.byTestID('additional-printer-column-data-Status').should('contain.text', 'Ready')
})

after("after all tests", function () {
Operator.deleteFlowCollector()
cy.adminCLI(`oc adm policy remove-cluster-role-from-user cluster-admin ${Cypress.env('LOGIN_USERNAME')}`)
Expand Down
20 changes: 20 additions & 0 deletions web/cypress/views/flowcollector-status.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
export namespace flowcollectorStatusSelectors {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

these can be moved to netfow-page.ts instead of a new file

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think if Page is diff it should have a diff view

export const statusIndicator = '#flowcollector-status-indicator'
export const statusButton = 'button[aria-label="FlowCollector status"]'
export const statusTooltip = '#flowcollector-status-tooltip'
export const readyRow = '[id=Ready-row]'
export const agentReadyRow = '[id=WaitingEBPFAgents-row]'
export const pluginReadyRow = '[id=WaitingWebConsole-row]'
export const monitoringReadyRow = '[id=WaitingMonitoring-row]'
export const configIssueRow = '[id=ConfigurationIssue-row]'
export const flpMonolithRow = '[id=WaitingFLPMonolith-row]'
export const lokiStackRow = '[id=WaitingLokiStack-row]'
export const flpParentRow = '[id=WaitingFLPParent-row]'
}

export const flowcollectorStatusPage = {
visit: () => {
cy.visit('k8s/cluster/flows.netobserv.io~v1beta2~FlowCollector/status')
cy.get(flowcollectorStatusSelectors.readyRow, { timeout: 30000 }).should('exist')
}
}
13 changes: 10 additions & 3 deletions web/cypress/views/netobserv.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ type FlowCollectorParameter =
| 'DNSTracking'
| 'UDNMapping'
| 'LokiDisabled'
| 'LokiWithoutLokiStack'
| 'Conversations'
| 'ZonesAndMultiCluster'
| 'BytesMetrics'
Expand Down Expand Up @@ -48,6 +49,7 @@ const FIXTURE_PATHS = {
flowRTT: './cypress/fixtures/flowcollector/fc_flowRTT.yaml',
udnMapping: './cypress/fixtures/flowcollector/fc_UDN.yaml',
lokiDisabled: './cypress/fixtures/flowcollector/fc_lokiDisabled.yaml',
lokiWithoutLokiStack: './cypress/fixtures/flowcollector/fc_lokiWithoutLokiStack.yaml',
conversations: './cypress/fixtures/flowcollector/fc_conversations.yaml',
subnetLabels: './cypress/fixtures/flowcollector/fc_subnetLabel.yaml',
zonesMultiCluster: './cypress/fixtures/flowcollector/fc_zoneMulticluster.yaml',
Expand Down Expand Up @@ -165,6 +167,9 @@ export const Operator = {
case "LokiDisabled":
cy.deployFlowcollectorFromFixture(FIXTURE_PATHS.lokiDisabled)
break;
case "LokiWithoutLokiStack":
cy.deployFlowcollectorFromFixture(FIXTURE_PATHS.lokiWithoutLokiStack)
break;
case "Conversations":
cy.deployFlowcollectorFromFixture(FIXTURE_PATHS.conversations)
break;
Expand Down Expand Up @@ -199,11 +204,13 @@ export const Operator = {
// wait for all window refresh
cy.wait('@reload', { timeout: 100000 })
cy.log("Console refreshed successfully")
if (parameters !== "LokiDisabled") {
if (parameters !== "LokiDisabled" && parameters !== "LokiWithoutLokiStack") {
cy.adminCLI(`oc wait --for=condition=Ready pod -l app=loki -n ${project} --timeout=180s`)
}
Operator.visitFlowcollector()
cy.byTestID('status-text', { timeout: 120000 }).should('exist').should('contain.text', 'Ready')
if (parameters !== "LokiWithoutLokiStack") {
Operator.visitFlowcollector()
cy.byTestID('status-text', { timeout: 120000 }).should('exist').should('contain.text', 'Ready')
}
}
})
},
Expand Down
27 changes: 27 additions & 0 deletions web/cypress/views/search.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
export const searchPage = {
navToSearchPage: () => cy.visit('/search/all-namespaces'),
chooseResourceType: (resource_type) => {
cy.get('input[placeholder="Resources"]').clear().type(`${resource_type}`);
cy.get(`label[id$="~${resource_type}"]`).click();
},
checkNoMachineResources: () => {
searchPage.navToSearchPage();
cy.get('[placeholder="Resources"]').type("machine");
const machineResources = ['MMachine','MAMachineAutoscaler','MCMachineConfig','MCPMachineConfigPool','MHCMachineHealthCheck','MSMachineSet'];
machineResources.forEach((machineResource) => {
cy.get(`[data-filter-text=${machineResource}]`).should('not.exist');
});
},
clearAllFilters: () => {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This func already exists know?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I just copied the search view from openshfit-test-private repo and this function is for search view.
We have to think about how we will manage views that is not related to netobserv

cy.byButtonText('Clear all filters').click({force: true});
},
searchMethodValues: (method, value) => {
method = method.toLocaleLowerCase();
cy.get('button[id="search-filter-toggle"]').click();
cy.get(`li[data-test="${method}-filter"] button[role="option"]`).click();
cy.get('input[id="search-filter-input"]').clear().type(`${value}`);
},
Comment on lines +18 to +23
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Prefer toLowerCase() over toLocaleLowerCase() for selector construction.

toLocaleLowerCase() depends on the runtime locale (e.g., Turkish locale maps Iı), which can produce a selector like li[data-test="ı-filter"] and silently fail. Since this string is used to construct a DOM selector, use locale-independent toLowerCase().

-    method = method.toLocaleLowerCase();
+    method = method.toLowerCase();
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
searchMethodValues: (method, value) => {
method = method.toLocaleLowerCase();
cy.get('button[id="search-filter-toggle"]').click();
cy.get(`li[data-test="${method}-filter"] button[role="option"]`).click();
cy.get('input[id="search-filter-input"]').clear().type(`${value}`);
},
searchMethodValues: (method, value) => {
method = method.toLowerCase();
cy.get('button[id="search-filter-toggle"]').click();
cy.get(`li[data-test="${method}-filter"] button[role="option"]`).click();
cy.get('input[id="search-filter-input"]').clear().type(`${value}`);
},
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/cypress/views/search.ts` around lines 18 - 23, In searchMethodValues
replace the locale-dependent call by using method = method.toLowerCase() instead
of method.toLocaleLowerCase() so the DOM selector construction (used in
li[data-test="${method}-filter"]) is locale-independent; update the assignment
in the searchMethodValues function accordingly.

searchBy: (text) => {
cy.get('input[data-test-id="item-filter"]').clear().type(`${text}`)
},
}
Loading