fix/argo #133

wjro · 2025-09-05T15:39:14Z

wjro commented

2025-09-05 15:39:14 +00:00

No description provided.

wjro added 8 commits 2025-09-05 15:39:14 +00:00

feat(example): added an example of packaging a rust app from github

Run Check Script / check (pull_request) Successful in 1m0s

Details

aebde3a382

fix: cache works, modified tar building function to ignore adding autogenerated files from harmony, .git, .github, target, and node_modules file paths\

Run Check Script / check (pull_request) Successful in 54s

Details

32ffc3ef61

> tar building function orders files and sets mtime to zero so it is consistent across docker image builds

Merge remote-tracking branch 'origin/master' into example/tryrust-webapp

Run Check Script / check (pull_request) Failing after 7s

Details

85d7baac6c

fix:deploys namespaced grafana operator in all cases

Run Check Script / check (pull_request) Failing after -4s

Details

95c2e1dd72

fix:cargo

Run Check Script / check (pull_request) Failing after -5s

Details

c6041bc154

fix: duplicate cargo entries

Run Check Script / check (pull_request) Failing after 16s

Details

8e5e674124

fix: example url and cargo incorrect

Run Check Script / check (pull_request) Successful in 59s

Details

fde0cb0a0f

fix: argo cd uses the correct hostname for route, helmchart is not hardcoded

Run Check Script / check (pull_request) Successful in 59s

Details

d13a23c001

letian reviewed 2025-09-05 16:46:55 +00:00

harmony/src/modules/application/rust.rs

						
				@ -59,6 +61,7 @@ pub struct RustWebapp {

				    pub domain: Url,

				    /// The path to the root of the Rust project to be containerized.

				    pub project_root: PathBuf,

				    pub service_port: u32,

letian commented

2025-09-05 16:46:55 +00:00

Is there a way to not have to provide this? It seems to me that the end user shouldn't have to worry about it and that it should be handled by the topology.

letian reviewed 2025-09-05 19:20:48 +00:00

harmony/src/modules/application/features/continuous_delivery.rs Outdated

						
				@ -176,18 +176,18 @@ impl<

				            }

				            target => {

				                info!("Deploying {} to target {target:?}", self.application.name());

				                info!("application name {}", self.application.name());

letian commented

2025-09-05 19:20:48 +00:00

The info above wasn't enough? It's already logging the application.name

The info above wasn't enough? It's already logging the `application.name`

letian marked this conversation as resolved

letian reviewed 2025-09-05 19:35:30 +00:00

harmony/src/modules/application/features/helm_argocd_score.rs

						
				@ -88,0 +103,4 @@

				    ) -> Result<String, InterpretError> {

				        //This should be the job of the topology to determine if we are in

				        //openshift, potentially we need on openshift topology the same way we create a

				        //localhosttopology

letian commented

2025-09-05 19:35:30 +00:00

It's probably a bit too much to fix it now, but indeed the Topology should provide the capability to get the host domain. Actually it should probably provide a higher level capability: create domain for a service (<-- example of a Tell Don't Ask).

For example:

let domain = topology.map_domain("argo");
let helm_score = argo_helm_chart_score(&self.score.namespace, self.score.openshift, &domain);

It's probably a bit too much to fix it now, but indeed the Topology should provide the capability to get the host domain. Actually it should probably provide a higher level capability: create domain for a service (<-- example of a Tell Don't Ask). For example: ```rs let domain = topology.map_domain("argo"); let helm_score = argo_helm_chart_score(&self.score.namespace, self.score.openshift, &domain); ```

letian reviewed 2025-09-05 19:37:36 +00:00

harmony/src/modules/prometheus/k8s_prometheus_alerting_score.rs Outdated

						
				@ -205,0 +202,4 @@

				        //     return Ok(Outcome::success("Grafana CRDs already exist".to_string()));

				        // }

				        //may not need to check this since the grafana operator is namespaceScope=true -> it

				        //will deploy with rolebindings rather than cluster role bindings