Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correct Kubernetes versions for multi-primary scenarios #7370

Merged
merged 3 commits into from
May 22, 2024

Conversation

ferhoyos
Copy link
Contributor

@ferhoyos ferhoyos commented May 21, 2024

Describe the change

This PR adds support for multi-primary scenarios, providing correct versions for Kubernetes in the mesh page.

Steps to test the PR

  1. Deploy a multi-primary scenario
  2. Go to Mesh page
  3. Verify that Kubernetes versions are the correct ones

image

image

Automation testing

N/A

Issue reference

Fixes #7361

@ferhoyos ferhoyos self-assigned this May 21, 2024
@ferhoyos ferhoyos added the test: n/a PR does not need test additions or updates label May 21, 2024
@ferhoyos ferhoyos changed the title Correct Kubernetes and Istio versions for multi-primary scenarios Correct Kubernetes versions for multi-primary scenarios May 21, 2024
@ferhoyos
Copy link
Contributor Author

ferhoyos commented May 21, 2024

Currently the Istio version from remote cluster cannot be obtained because Istio settings are only supported for single-cluster (related to issue #7203). I think that for the moment we can fix only Kubernetes version and assume that remote Istio version is the same as home cluster one.

@ferhoyos ferhoyos marked this pull request as ready for review May 21, 2024 15:50
@ferhoyos ferhoyos requested review from nrfox and jshaughn May 21, 2024 15:51
Copy link
Collaborator

@jshaughn jshaughn left a comment

Choose a reason for hiding this comment

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

Tested on my single OCP 4.14 CRC cluster and working fine in that scenario. I'll leave the changes to version determination to Nick as I know you two were going back on forth on it. +1 to limiting to kube version. Also, the CI fails are all know flakes so no problem there. LGTM.

image

@@ -65,6 +65,7 @@
"Kiali home cluster": "Kiali home cluster",
"Kiali home cluster: {{name}}": "Kiali主集群: {{name}}",
"Kiali on GitHub": "Kiali on GitHub",
"Kubernetes": "Kubernetes",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess this maybe has a translation into other character sets? I don't know, not a problem to include it even if they leave it alone.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know, probably there is no translation. I just changed Version to Kubernetes and I did not care about the translation. Anyway, if that is the case we can remove it afterwards.

{renderNodeHeader(clusterData, { nameOnly: true, smallSize: false, hideBadge: clusterData.isExternal })}
<div className={infoStyle}>
{!clusterData.isExternal && `${t('Version')}: ${clusterData.version || UNKNOWN}`}
{!clusterData.isExternal && `${t('Kubernetes')}: ${clusterData.version || UNKNOWN}`}
Copy link
Collaborator

Choose a reason for hiding this comment

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

+1, this is better.

@@ -62,7 +65,7 @@ export const TargetPanelMesh: React.FC<TargetPanelMeshProps> = (props: TargetPan

const renderInfraNodeSummary = (nodeData: MeshNodeData): React.ReactNode => {
return (
<div className={summaryStyle}>
<div key={nodeData.id} className={summaryStyle}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

+1, avoid key conflict.

@jshaughn
Copy link
Collaborator

Merging as CI failures are known flakes and @ferhoyos has said it is good to merge.

@jshaughn jshaughn merged commit f5f9437 into kiali:master May 22, 2024
7 of 9 checks passed
@ferhoyos ferhoyos deleted the 7361-correct-mesh-versions branch August 1, 2024 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test: n/a PR does not need test additions or updates
Projects
Development

Successfully merging this pull request may close these issues.

[mesh page] show correct cluster versions in multi-cluster deployments
2 participants