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

fix(enter): block --verbose for exec subcommand #394

Merged
merged 2 commits into from
Jun 30, 2024

Conversation

rebornplusplus
Copy link
Member

@rebornplusplus rebornplusplus commented Mar 21, 2024

NOTE: do not merge until canonical/rockcraft#495 is finalized.

This PR blocks the usage of --verbose option in "enter" command whenever the "exec" subcommand follows. Thus, the following will result in errors:

pebble enter --verbose [enter-OPTS*] exec [exec-OPTS] <cmd..>

Everything else, however, are kept unchanged. So the following is okay:

pebble enter [enter-OPTS*] exec [exec-OPTS] <cmd..>

Note that enter-OPTS* refer to all "enter" options except -v, --verbose.

Resolves #339.

This commit blocks the usage of ``--verbose`` option in "enter" command
whenever the "exec" subcommand follows. Thus, the following will result
in errors:

    pebble enter --verbose [enter-OPTS*] exec [exec-OPTS] <cmd..>

Everything else, however, are kept unchanged. So the following is okay:

    pebble enter [enter-OPTS*] exec [exec-OPTS] <cmd..>

Note that enter-OPTS* refers to all "enter" options except -v,
--verbose.

Originated from canonical#339.
@rebornplusplus
Copy link
Member Author

@linostar, please let me know if this PR should block on any Rockcraft decisions.

@linostar
Copy link

@linostar, please let me know if this PR should block on any Rockcraft decisions.

Please hold until canonical/rockcraft#495 is finalised.

@benhoyt
Copy link
Contributor

benhoyt commented Mar 21, 2024

For my benefit, what controls which version of Pebble is baked into a Rock? Is that dependent on the Rockcraft version, that is, Rockcraft version X corresponds to Pebble version Y?

@rebornplusplus
Copy link
Member Author

rebornplusplus commented Mar 22, 2024

It's always the latest stable snap of pebble: https://github.com/canonical/rockcraft/blob/main/rockcraft/pebble.py#L167-L180

    PEBBLE_PATH = "var/lib/pebble/default"
    PEBBLE_LAYERS_PATH = f"{PEBBLE_PATH}/layers"
    PEBBLE_BINARY_PATH = "bin/pebble"
    PEBBLE_PART_SPEC = {
        "plugin": "nil",
        "stage-snaps": ["pebble/latest/stable"],
        "stage": [PEBBLE_BINARY_PATH],
        # We need this because "services" is Optional, but the directory must exist
        "override-prime": str(
            "craftctl default\n"
            f"mkdir -p {PEBBLE_LAYERS_PATH}\n"
            f"chmod 777 {PEBBLE_PATH}"
        ),
    }

Copy link
Contributor

@benhoyt benhoyt left a comment

Choose a reason for hiding this comment

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

Approved with two nit suggestions (though we won't merge until l canonical/rockcraft#495 is ready).

@rebornplusplus
Copy link
Member Author

Thanks! I have updated the error messages. But yes, let's please hold this until canonical/rockcraft#495 is ready.

@cjdcordeiro
Copy link
Collaborator

@rebornplusplus why do we need to wait for canonical/rockcraft#495?

@rebornplusplus
Copy link
Member Author

@rebornplusplus why do we need to wait for canonical/rockcraft#495?

Since rockcraft fetches the latest pebble binary/snap, docker run <rock> exec <cmd> would fail if --verbose is not removed from the rock's entrypoint.

@cjdcordeiro
Copy link
Collaborator

y true. thanks

@benhoyt
Copy link
Contributor

benhoyt commented Jun 11, 2024

Now that canonical/rockcraft#495 is merged, can we merged this? (We won't release a new Pebble version till the end of June, though.) Or do we need to wait till a new version of rockcraft is shipped?

@cjdcordeiro
Copy link
Collaborator

We better wait for a new rockcraft release, otherwise, there's a chance this change gets to stable before rockcraft, and those using rockcraft will immediately become unable to run the exec command for their rocks.

@tigarmo what's the planned date for releasing a new v of rockcraft?

@cjdcordeiro
Copy link
Collaborator

rockcraft 1.4.0 is now in stable. Let's allow for a graceful week and merge this on the 27th.

@benhoyt
Copy link
Contributor

benhoyt commented Jun 20, 2024

Sounds good, thanks @cjdcordeiro!

@cjdcordeiro
Copy link
Collaborator

We should be good to go on this @benhoyt ! Thanks for waiting

@benhoyt benhoyt merged commit 21916a6 into canonical:master Jun 30, 2024
15 checks passed
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.

[bug] pebble "enter" keeps logging, with unexpected indentation, when combined with "exec"
4 participants