Skip to content

Conversation

mrsimonemms
Copy link
Contributor

@mrsimonemms mrsimonemms commented Jul 8, 2022

Description

Applies tests on the CPU/memory to the workspace nodes only. Our requirements only stipulate the performance of these nodes, not all the nodes in the cluster.

This also changes the wording for the CPU tests to change "cluster" to "node" (a remnant of when we had the tests on a cluster basis).

Related Issue(s)

Fixes #9307

How to test

Run the preflight checks

Release Notes

[kots]: add node CPU/memory check tests to workspace node only

Documentation

Werft options:

  • /werft with-preview

@mrsimonemms mrsimonemms force-pushed the sje/kots-memory-checks branch from fe1cf43 to cc9124a Compare July 8, 2022 13:25
@mrsimonemms mrsimonemms marked this pull request as ready for review July 8, 2022 14:24
@mrsimonemms mrsimonemms requested a review from a team July 8, 2022 14:24
@github-actions github-actions bot added the team: delivery Issue belongs to the self-hosted team label Jul 8, 2022
@lucasvaltl
Copy link
Contributor

lucasvaltl commented Jul 11, 2022

👋 Do we have a minimum requirement for the non-workspace nodes though? I fear that there might be a case where someone uses an underpowered node for the server component for example and then wonders why. We did have another related issue (#10867) asking for a per-node label hardware requirement. How difficult would that be to implement?

@mrsimonemms
Copy link
Contributor Author

Honestly, I don't know what the requirements for the non-workspace nodes are. To the best of my knowledge, these aren't documented anywhere so one assumes that there aren't any explicit requirements.

I think these two tickets are pretty much a duplication of each other, albeit with a slightly different focus (#10867 is about the general required whereas #9307 is about how this is tested in KOTS).

The implementation for #10867 is very easy once we have the requirements. My feeling is that that ticket has three phases:

  1. Get the requirements from the specific teams
  2. Add this into the documentation
  3. Add this into KOTS (this will end up looking very similar to this PR, except with other node labels)

Copy link
Contributor

@nandajavarma nandajavarma left a comment

Choose a reason for hiding this comment

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

Changes look good! Didn't test it.

@roboquat roboquat merged commit f992eaf into main Jul 13, 2022
@roboquat roboquat deleted the sje/kots-memory-checks branch July 13, 2022 07:58
@lucasvaltl
Copy link
Contributor

Did we want to merge this already?

@mrsimonemms
Copy link
Contributor Author

@lucasvaltl I think it's fine that this is merged in as it was technically incorrect (see #9307). We can raise the additional things as a future PR to make it even more correct

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note size/M team: delivery Issue belongs to the self-hosted team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Investigate KOTS Preflight Checks Reporting Cluster Memory Incorrectly
4 participants