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

Bring forward SPS marketplace adjustments #245

Merged
merged 9 commits into from
Dec 10, 2024

Conversation

jpl-btlunsfo
Copy link
Collaborator

Purpose

  • Bring the extra bits SPS requires to safely install from the Unity Marketplace into the develop branch.

Proposed Changes

  • ADD some extra variables (unused) required by the management console's installation procedures
  • CHANGE some of the module source references to work inside the management console (ssh vs https fetch issues)

Issues

Testing

apparently have been calling the wrong lambda the whole time
- adjusting eks module source branch
- adding project/venue variables as args to eks
- adding management-console required variables
- fixing sps initiators module sources
- adding mc-required variables to airflow terraform
@jpl-btlunsfo jpl-btlunsfo self-assigned this Dec 9, 2024
@jpl-btlunsfo
Copy link
Collaborator Author

Just to note, this PR is actually also including the changes from #228, as they're necessary for the installation's proxies (though #228's changes arent merged yet)

Copy link
Collaborator

@LucaCinquini LucaCinquini left a comment

Choose a reason for hiding this comment

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

Hi @jpl-btlunsfo : I was able to deploy from this branch and execute the DAGs successfully. Only question: what are the new parameters needed by the Management Console? In particular, can we set "deployment_name" to be the same as "project", so we don't have to define the same value twice for 2 different variables?

@jpl-btlunsfo
Copy link
Collaborator Author

what are the new parameters needed by the Management Console

The management console, when installing terraform modules/applications, does a really odd force-injection of variables here. I think the terraform call includes some 'strict' setting, as the absence of any of the variables included there in the actual terraform config throws a fatal error (in the management console output at least).

can we set "deployment_name" to be the same as "project", so we don't have to define the same value twice for 2 different variables

deployment_name is never actually used by the SPS terraform configuration (nor are tags or install_prefix). It does get injected by the management console though?

I guess we could set its default to something simple (like a "" empty string or something), but it definitely needs to be its own separate variable, if only to keep the management console's installer code happy.

@LucaCinquini
Copy link
Collaborator

@jpl-btlunsfo : ok, can you set the default value of "deployment_name" to be ""? Then we can merge these changes.

@jpl-btlunsfo jpl-btlunsfo marked this pull request as ready for review December 9, 2024 19:19
@jpl-btlunsfo
Copy link
Collaborator Author

tag updated, just need an approval to merge (will hold merging until I run another test-deploy).

Copy link
Collaborator

@LucaCinquini LucaCinquini left a comment

Choose a reason for hiding this comment

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

Thanks, approved.

@jpl-btlunsfo
Copy link
Collaborator Author

jpl-btlunsfo commented Dec 10, 2024

@jpl-btlunsfo jpl-btlunsfo merged commit 433373d into develop Dec 10, 2024
2 checks passed
@jpl-btlunsfo jpl-btlunsfo deleted the 59-marketplace-adjustments-edge branch December 10, 2024 04:13
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.

2 participants