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.

Sign up for GitHub

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

Jump to bottom

[ENH] - Include "--attempt-fixes" flag from Nebari upgrade CLI in upgrade steps logic #2761

Closed
viniciusdc opened this issue Oct 9, 2024 · 5 comments · Fixed by #2839
Closed

[ENH] - Include "--attempt-fixes" flag from Nebari upgrade CLI in upgrade steps logic #2761

viniciusdc opened this issue Oct 9, 2024 · 5 comments · Fixed by #2839
Assignees
@smokestacklightnin
Labels
area: CI/CD 👷🏽‍♀️ area: nebari-cli good first issue Good for newcomers
Milestone
2025.1.1 release

Comments

@viniciusdc
Copy link
Contributor

viniciusdc commented Oct 9, 2024 •
edited by marcelovilla
Loading

Feature description

We are currently exposing an --attempt-fixes flag in the nebari upgrade subcommand:

nebari/src/_nebari/subcommands/upgrade.py

Lines 19 to 23 in 9b1310b

attempt_fixes: bool = typer.Option(
False,
"--attempt-fixes",
help="Attempt to fix the config for any incompatibilities between your old and new Nebari versions.",
),

This parameter can be accessed from the UpgradeStep class, where we define specific upgrade logic for each upgrade step between contiguous versions:

nebari/src/_nebari/upgrade.py

Lines 52 to 111 in 9b1310b

def do_upgrade(config_filename, attempt_fixes=False):
"""
Perform an upgrade of the Nebari configuration file.
This function loads the YAML configuration file, checks for deprecated keys,
validates the current version, and if necessary, upgrades the configuration
to the latest version of Nebari.
Args:
config_filename (str): The path to the configuration file.
attempt_fixes (bool): Whether to attempt automatic fixes for validation errors.
Returns:
None
"""
config = load_yaml(config_filename)
if config.get("qhub_version"):
rich.print(
f"Your config file [purple]{config_filename}[/purple] uses the deprecated qhub_version key. Please change qhub_version to nebari_version and re-run the upgrade command."
)
return
try:
from nebari.plugins import nebari_plugin_manager
nebari_plugin_manager.read_config(config_filename)
rich.print(
f"Your config file [purple]{config_filename}[/purple] appears to be already up-to-date for Nebari version [green]{__version__}[/green]"
)
return
except (ValidationError, ValueError) as e:
if is_version_accepted(config.get("nebari_version", "")):
# There is an unrelated validation problem
rich.print(
f"Your config file [purple]{config_filename}[/purple] appears to be already up-to-date for Nebari version [green]{__version__}[/green] but there is another validation error.\n"
)
raise e
start_version = config.get("nebari_version", "")
UpgradeStep.upgrade(
config, start_version, __version__, config_filename, attempt_fixes
)
# Backup old file
backup_configuration(config_filename, f".{start_version or 'old'}")
with config_filename.open("wt") as f:
yaml.dump(config, f)
rich.print(
f"Saving new config file [purple]{config_filename}[/purple] ready for Nebari version [green]{__version__}[/green]"
)
ci_cd = config.get("ci_cd", {}).get("type", "")
if ci_cd in ("github-actions", "gitlab-ci"):
rich.print(
f"\nSince you are using ci_cd [green]{ci_cd}[/green] you also need to re-render the workflows and re-commit the files to your Git repo:\n"
f" nebari render -c [purple]{config_filename}[/purple]\n"
)

However, we're not using this flag in the most recent upgrade steps. We need to ensure that upgrade steps use this flag and define a logical path to upgrade without user input.

Value and/or benefit

Recently, #2780 got merged, implementing the logic to add automatic upgrade tests to our GHA workflows. Because tests are running automatically, we cannot rely on user input, so they are relying on passing the --attempt-fixes flag when calling the nebari upgrade command:

nebari/.github/workflows/test_local_upgrade.yaml

Line 73 in 364c9e3

nebari upgrade --config ${{ steps.init.outputs.config }} --attempt-fixes

Anything else?

No response
@Suraj-kumar00
Copy link

Hi @viniciusdc, how have you been?
As I'm working on DevOps practices, I would love to work on this issue but can you please give me some more context?
What I have understood from you description is that:

  • The --force flag will automatically accept upgrade prompts, bypassing manual confirmation.
  • It is useful for CI pipelines to automate the upgrade process without user intervention.
  • It speeds up testing and ensures upgrades can proceed in automated environments.
  • Overall, it streamlines CI/CD processes by allowing the upgrade command to run without manual input.

@marcelovilla
Copy link
Member

marcelovilla commented Oct 24, 2024 •
edited
Loading

Hey @Suraj-kumar00!

Sorry for the late reply. Your interest in this issue is very welcome!

  • The --force flag will automatically accept upgrade prompts, bypassing manual confirmation.
  • It is useful for CI pipelines to automate the upgrade process without user intervention.
  • It speeds up testing and ensures upgrades can proceed in automated environments.
  • Overall, it streamlines CI/CD processes by allowing the upgrade command to run without manual input.

Your understanding is correct. If you take a look at the upgrade.py file, you'll notice that we have an upgrade step for each version, where we prompt the user depending on what needs to be done. While there are some cases where no action is required from the user, there are other cases where the user needs to decide whether they should nebari to run operations on their behalf. See, for example,

nebari/src/_nebari/upgrade.py

Lines 1032 to 1036 in 215bbd6

run_commands = Prompt.ask(
"\nDo you want Nebari to update the kube-prometheus-stack CRDs and delete the prometheus-node-exporter for you? If not, you'll have to do it manually.",
choices=["y", "N"],
default="N",
)

The idea of having a --force flag is to clearly define what the default behavior should be, without any user input, to upgrade Nebari. That will allow us to run automatic upgrade tests in our CI/CD workflows and improve our confidence in the changes we incorporate.

There's currently a PR ready to be reviewed implementing the logic to add these automatic upgrade tests to our GHA workflows. You'll notice it's actually using the nebari upgrade command and passing the --attempt-fixes flag:

nebari/.github/workflows/test_local_upgrade.yaml

Line 73 in 09e34a5

nebari upgrade --config ${{ steps.init.outputs.config }} --attempt-fixes

This would be the equivalent of having a --force flag and is already implemented and fully exposed to the upgrade steps. For example:

nebari/src/_nebari/upgrade.py

Lines 485 to 494 in 3f2523a

if not kwargs.get("attempt_fixes", False):
raise ValueError(
f"{customauth_warning}\n\nRun `nebari upgrade --attempt-fixes` to switch to basic Keycloak authentication instead."
)
else:
rich.print(f"\nWARNING: {customauth_warning}")
rich.print(
"\nSwitching to basic Keycloak authentication instead since you specified --attempt-fixes."
)
config["security"]["authentication"] = {"type": "password"}

Now, what we need to do, is to make sure that recent upgrade steps that prompt the user for input can be run automatically by defining a default logical path using this flag.

Let me know if you're still interested in contributing a fix for this issue and I'll be happy to answer any questions you might have.

@viniciusdc does it make sense to edit the issue body and title to reflect the fact that we don't need an extra flag as we already have one implemented? It's just a matter of actually using it on the existing and coming upgrade steps.

@viniciusdc
Copy link
Contributor Author

@viniciusdc does it make sense to edit the issue body and title to reflect the fact that we don't need an extra flag as we already have one implemented? It's just a matter of actually using it on the existing and coming upgrade steps.

Yes, I think it does. The renaming will still be kept in history if needed.

@marcelovilla marcelovilla added this to the 2024.11.2 release milestone Nov 1, 2024
@marcelovilla
Copy link
Member

Hey @Suraj-kumar00. Just reaching out to see if you're still interested in picking up this issue.

@marcelovilla marcelovilla changed the title [ENH] - Include "force" flag to upgrade command CLI [ENH] - Include "--attempt-fixes" flag from Nebari upgrade CLI in upgrade steps logic Nov 5, 2024
@marcelovilla
Copy link
Member

marcelovilla commented Nov 8, 2024 •
edited
Loading

@smokestacklightnin, why don't you go ahead and pick up this issue?

@smokestacklightnin smokestacklightnin mentioned this issue Nov 10, 2024
Merged
10 tasks
@github-project-automation github-project-automation bot moved this from New 🚦 to Done 💪🏾 in 🪴 Nebari Project Management Jan 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@smokestacklightnin smokestacklightnin

Labels
area: CI/CD 👷🏽‍♀️ area: nebari-cli good first issue Good for newcomers
Projects
🪴 Nebari Project Management
Status: Done 💪🏾
Milestone
2025.1.1 release
4 participants
@marcelovilla @viniciusdc @Suraj-kumar00 @smokestacklightnin