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

Get sysdicloud-values secret #238

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

BuenClima
Copy link

Now if the backend version is higher than 6.5, it should get the sysdigcloud-values secret which include different settings such as JVM, etc...

Now if the backend version is higher than 6.5, it should get the sysdigcloud-values secret which include different settings such as JVM    etc...
@BuenClima BuenClima self-assigned this May 31, 2024
@BuenClima BuenClima changed the title Update get_support_bundle.sh Get sysdicloud-values secret May 31, 2024
Copy link

@jameslarrea jameslarrea left a comment

Choose a reason for hiding this comment

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

Thanks @BuenClima! Looks good but the namespace should not be hardcoded because while the default the installer uses is sysdigcloud the customer can use any name. It gets set in the script here

KUBE_OPTS="--namespace ${NAMESPACE} ${CONTEXT_OPTS}"
You can see the other kubectl commands for examples.
Also, minor but can we name the values file where this data will be stored imported_values.yaml?

@BuenClima BuenClima requested a review from jameslarrea May 31, 2024 13:17
@jameslarrea jameslarrea requested a review from mbreitung May 31, 2024 13:28
jameslarrea
jameslarrea previously approved these changes May 31, 2024
Copy link

@jameslarrea jameslarrea left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link

@jameslarrea jameslarrea left a comment

Choose a reason for hiding this comment

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

Not sure if the spacing inside the brackets will cause issues with some shells. Please update to show the spacing in all of them, like [[ $major -eq 6 ]]

@BuenClima BuenClima requested a review from jameslarrea June 17, 2024 08:33
Copy link

@jameslarrea jameslarrea left a comment

Choose a reason for hiding this comment

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

the variable $version is not defined. There is an issue with BACKEND_VERSION being fixed here #237
But even then, seems that variable is not global but rather we run it every time we need to check the version. Let me discuss with @dark-vex tomorrow if maybe we can refactor that part a bit so it's cleaner.

mbreitung
mbreitung previously approved these changes Jun 18, 2024
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.

3 participants