Skip to content

[minor] Add support For Installation and FVT setup for Aiservice 9.1.x #79

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

Open
wants to merge 12 commits into
base: stable
Choose a base branch
from

Conversation

Bhautik-Vala
Copy link

@Bhautik-Vala Bhautik-Vala commented Jun 3, 2025

ISSUES: MASAIB-7, MASAIB-761

Introduce new Command mas aiservice-install specifically for installation of aiservice 9.1.x or above, So Add PipelineRun ( pipelinerun-aiservice-install ) for aiservice-install Pipeline.

mas-aiservice-1 mas-aiservice-2 mas-aiservice-3

@Bhautik-Vala Bhautik-Vala changed the title [patch] Add support For Installation and FVT setup for Aiservice 9.1.x [minor] Add support For Installation and FVT setup for Aiservice 9.1.x Jun 3, 2025
@Bhautik-Vala Bhautik-Vala marked this pull request as ready for review June 3, 2025 08:19
Comment on lines 546 to 555
{%- if grafana_action is defined and grafana_action != "" %}
# Grafana
# -------------------------------------------------------------------------
- name: grafana_action
value: "{{ grafana_action }}"
- name: grafana_v5_namespace
value: "{{ grafana_v5_namespace }}"
- name: grafana_instance_storage_size
value: "{{ grafana_instance_storage_size }}"
{%- endif %}
Copy link
Member

Choose a reason for hiding this comment

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

is grafana part of aibroker?

Comment on lines 539 to 544
# MAS Workspace
# -------------------------------------------------------------------------
- name: mas_workspace_id
value: "{{ mas_workspace_id }}"
- name: mas_workspace_name
value: "{{ mas_workspace_name }}"
Copy link
Member

Choose a reason for hiding this comment

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

there is a mas workspace?

- name: artifactory_token
value: "{{ artifactory_token }}"
{%- endif %}
{%- if ibmcloud_apikey is defined and ibmcloud_resourcegroup != "" %}
Copy link
Member

Choose a reason for hiding this comment

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

A large portion of these variables don;'t seem to be related to aibroker? Are these all needed?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, this looks like a copy/paste of the mas install pipeline run ... this needs to be fixed to only be the params used by aiservice install

apiVersion: tekton.dev/v1beta1
kind: PipelineRun
metadata:
name: "{{mas_instance_id}}-install-{{ timestamp }}"
Copy link
Member

Choose a reason for hiding this comment

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

can the pipelinerun name be aiservice-install? Also why is mas_instance_id related to this?

Comment on lines 406 to 413
"""
Create a PipelineRun to install the chosen MAS instance (and selected dependencies)
"""
instanceId = params["mas_instance_id"]
namespace = f"mas-{instanceId}-pipelines"
timestamp = launchPipelineRun(dynClient, namespace, "pipelinerun-aiservice-install", params)

pipelineURL = f"{getConsoleURL(dynClient)}/k8s/ns/mas-{instanceId}-pipelines/tekton.dev~v1beta1~PipelineRun/{instanceId}-install-{timestamp}"
Copy link
Member

Choose a reason for hiding this comment

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

Not sure why the mas_instance_id is related to this.

Copy link
Author

Choose a reason for hiding this comment

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

we have the logic in cli which generate namspace for pipeline using mas_instance_id combination so that's why have to use this params and also in some role of aiservice also we are using mas_instance_id as env var so we need this var.

Copy link
Member

Choose a reason for hiding this comment

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

@Bhautik-Vala the issue is that the variable name is wrong. You would be using the value of this to be the aiservice instance id not the mas instanceid.

@@ -77,7 +77,9 @@ mas_visualinspection_version:
9.0.x: 9.0.8 # updated
8.10.x: 8.8.4 # No Update
8.11.x: 8.9.11 # updated

mas_aibroker_version:
Copy link
Contributor

Choose a reason for hiding this comment

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

aibroker or aiservice, I thought we were using the latter now?

Create a PipelineRun to install the Aiservice
"""
instanceId = params["mas_instance_id"]
namespace = f"mas-{instanceId}-pipelines"
Copy link
Contributor

Choose a reason for hiding this comment

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

Unless the aiservice is part of the mas instance, it should not be using the mas-instanceid-pipelines namespace. This is only for pipelines that are scoped to the mas instance.

e.g.

  • we use mas-pipelines namespace for the cluster-scoped update
  • we use mas-instanceid-pipelines for the instance-scoped upgrade, install, and uninstall

- name: artifactory_token
value: "{{ artifactory_token }}"
{%- endif %}
{%- if ibmcloud_apikey is defined and ibmcloud_resourcegroup != "" %}
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, this looks like a copy/paste of the mas install pipeline run ... this needs to be fixed to only be the params used by aiservice install

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

Successfully merging this pull request may close these issues.

4 participants