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

feat: onboard argo apps in devtron #2428

Open
wants to merge 38 commits into
base: develop
Choose a base branch
from

Conversation

AbhishekA1509
Copy link
Member

@AbhishekA1509 AbhishekA1509 commented Jan 30, 2025

Description

This PR includes changes to add functionality to onboard external argo apps in devtron application.

Fixes https://github.com/devtron-labs/sprint-tasks/issues/1616

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

QA

Checklist:

  • The title of the PR states what changed and the related issues number (used for the release note).
  • Does this PR require documentation updates?
  • I've updated documentation as required by this PR.
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas

…_EXTERNAL_APPS in SourceInfo, BuildCD, and CDPipeline components
…or, cdPipeline, cluster, and navigation components
…nt in BuildCD and NavigationRoutes components
…g, dynamic content rendering, and new styling for info bars
…improved error messaging, and icon color adjustment
…nd enhance edit functionality with button updates
…ce MigrateFromArgo with option value handling, and update ClusterForm and ClusterEnvironmentDrawer for local storage integration
…igrateToDevtronValidation components, and add common props for error display
…n MigrateToDevtronValidationFactory for improved user experience
@github-actions github-actions bot added the PR:Issue-verification-failed PR:Issue-verification-failed label Feb 3, 2025
@github-actions github-actions bot removed the PR:Issue-verification-failed PR:Issue-verification-failed label Feb 3, 2025
@github-actions github-actions bot added the PR:Ready-to-Review PR:Ready-to-Review label Feb 3, 2025
Comment on lines 121 to 133
.border-top {
&__primary {
border-top: 1px solid var(--border-primary);
}

&__secondary {
border-top: 1px solid var(--border-secondary);
}

&__white {
border-top: 1px solid var(--white);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

instead we should have border__primary--top and so on

@@ -91,3 +94,31 @@ export const createTaintsList = (list: any[], nodeLabel: string): Map<string, No
return taints
}, new Map<string, NodeTaintType[]>())
}

export const getServerURLFromLocalStorage = (fallbackServerUrl: string): string => {
if (typeof Storage !== 'undefined' && localStorage.getItem(ADD_CLUSTER_FORM_LOCAL_STORAGE_KEY)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

  • Let's not use the Storage undefined check.
  • DRY the read from local storage into variable for both instances

Comment on lines 323 to 342
render={(props) => {
const clusterName = props.match.params.clusterName
const { isVirtualCluster, prometheus_url, id: clusterId } =
this.state.clusters.find((cluster) => cluster.cluster_name === clusterName) || {}

return (
<ClusterEnvironmentDrawer
reload={this.initialise}
id={null}
environmentName={null}
clusterId={clusterId}
namespace={null}
prometheusEndpoint={prometheus_url}
isProduction={null}
description={null}
hideClusterDrawer={this.handleCloseCreateEnvironmentForm}
isVirtual={isVirtualCluster}
/>
)
}}
Copy link
Contributor

Choose a reason for hiding this comment

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

What if the clusterId is not available?

<div className="flexbox px-12 py-8 dc__gap-8 bcb-1 br-4 mb-16">
<ICInfo className="dc__no-shrink icon-dim-20 dc__no-shrink" />
<span className="fs-13 fw-4 lh-20 cn-9 dc__word-break">
This deployment pipeline was linked to{' '}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
This deployment pipeline was linked to{' '}
This deployment pipeline was linked to&nbsp;

import { GENERIC_SECTION_ERROR_STATE_COMMON_PROPS } from './constants'
import { generateArgoAppOption, generateClusterOption, sanitizeValidateMigrationSourceResponse } from './utils'

const RESOURCES_TO_FETCH: ResourceKindType.cluster[] = [ResourceKindType.cluster]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const RESOURCES_TO_FETCH: ResourceKindType.cluster[] = [ResourceKindType.cluster]
const RESOURCES_TO_FETCH: [ResourceKindType.cluster] = [ResourceKindType.cluster]

const clusterOptions = (resourcesOptionsMap?.[ResourceKindType.cluster] || [])
.filter((cluster) => !cluster.isVirtual)
.map<SelectClusterOptionType>((cluster) => generateClusterOption(cluster.name, cluster.id))
.sort((a, b) => stringComparatorBySortOrder(a.label as string, b.label as string))
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
.sort((a, b) => stringComparatorBySortOrder(a.label as string, b.label as string))

@@ -1371,7 +1372,8 @@ class GitOpsConfiguration extends Component<GitOpsProps, GitOpsState> {
const renderDirectoryManagementInGitOps = () => (
<div className="flex column left w-100 dc__gap-16 pb-16">
<div className="fw-6 cn-9 fs-14">Directory management in Git</div>
{window._env_.FEATURE_USER_DEFINED_GITOPS_REPO_ENABLE ? (
{window._env_.FEATURE_USER_DEFINED_GITOPS_REPO_ENABLE &&
this.props.isFeatureUserDefinedGitOpsEnabled ? (
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we try to incorporate FEATURE_USER_DEFINED_GITOPS_REPO_ENABLE also from BE?

@@ -1480,4 +1482,11 @@ class GitOpsConfiguration extends Component<GitOpsProps, GitOpsState> {
}
}

export default withRouter(GitOpsConfiguration)
const withIsFeatureUserDefinedGitOpsEnabled = (Component: ComponentType) => (props) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

instead of doing this we should have sent the value as prop from parent

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR:Ready-to-Review PR:Ready-to-Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants