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

WIP: Make rebooting a flag for "ob deploy push" #1036

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

WIP: Make rebooting a flag for "ob deploy push" #1036

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions lib/command/src/Obelisk/Command.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@
{-# LANGUAGE LambdaCase #-}
{-# LANGUAGE MultiParamTypeClasses #-}
{-# LANGUAGE OverloadedStrings #-}
{-# LANGUAGE PackageImports #-}
{-# LANGUAGE ScopedTypeVariables #-}
{-# LANGUAGE TupleSections #-}
{-# LANGUAGE PackageImports #-}
{- ORMOLU_DISABLE -}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think we should include formatting app specific pragmas.

Copy link
Member

Choose a reason for hiding this comment

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

Agree

module Obelisk.Command where

import Control.Monad.IO.Class (MonadIO, liftIO)
Expand Down Expand Up @@ -144,7 +145,7 @@ packageNames = some (strArgument (metavar "PACKAGE-NAME..."))
deployCommand :: ArgsConfig -> Parser DeployCommand
deployCommand cfg = hsubparser $ mconcat
[ command "init" $ info (DeployCommand_Init <$> deployInitOpts) $ progDesc "Initialize a deployment configuration directory"
, command "push" $ info (DeployCommand_Push <$> remoteBuilderParser) mempty
, command "push" $ info (DeployCommand_Push <$> remoteBuilderParser <*> reboot) mempty
, command "test" $ info (DeployCommand_Test <$> platformP) $ progDesc "Test your obelisk project from a mobile platform."
, command "update" $ info (pure DeployCommand_Update) $ progDesc "Update the deployment's src thunk to latest"
]
Expand All @@ -154,6 +155,9 @@ deployCommand cfg = hsubparser $ mconcat
, command "ios" $ info ((,) <$> pure IOS <*> fmap pure (strArgument (metavar "TEAMID" <> help "Your Team ID - found in the Apple developer portal"))) mempty
]

reboot :: Parser Bool
reboot = flag False True (long "reboot")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Rebooting on deploy is currently the default behaviour, this shouldn't change that. Otherwise users may be surprised to see that their running apps aren't the latest version following a deployment.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd argue that it should've never been turned on by default in the first place, This should've always been the users choice to actually reboot

Copy link
Member

Choose a reason for hiding this comment

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

You're talking about restarting the backend service, right?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I was mistaken this is referring to the restart of the machine the backend service is running on. The backend service will still be restarted as part of a deploy that did not include the --reboot flag.


remoteBuilderParser :: Parser (Maybe RemoteBuilder)
remoteBuilderParser =
flag (if enabledByDefault then enabled else Nothing) enabled (mconcat
Expand All @@ -171,6 +175,7 @@ deployCommand cfg = hsubparser $ mconcat
flagDesc = "managed Linux virtual machine as a Nix remote builder (requires Docker)"



deployInitOpts :: Parser DeployInitOpts
deployInitOpts = DeployInitOpts
<$> strArgument (action "directory" <> metavar "DEPLOYDIR" <> help "Path to a directory where the deployment repository will be initialized")
Expand All @@ -187,7 +192,7 @@ data RemoteBuilder = RemoteBuilder_ObeliskVM

data DeployCommand
= DeployCommand_Init DeployInitOpts
| DeployCommand_Push (Maybe RemoteBuilder)
| DeployCommand_Push (Maybe RemoteBuilder) Bool
| DeployCommand_Test (PlatformDeployment, [String])
| DeployCommand_Update
deriving Show
Expand Down Expand Up @@ -424,12 +429,12 @@ ob = \case
ObCommand_Init source force -> initProject source force
ObCommand_Deploy dc -> case dc of
DeployCommand_Init deployOpts -> withProjectRoot "." $ \root -> deployInit deployOpts root
DeployCommand_Push remoteBuilder -> do
DeployCommand_Push remoteBuilder reboot -> do
deployPath <- liftIO $ canonicalizePath "."
deployBuilders <- case remoteBuilder of
Nothing -> pure []
Just RemoteBuilder_ObeliskVM -> (:[]) <$> VmBuilder.getNixBuildersArg
deployPush deployPath deployBuilders
deployPush deployPath deployBuilders reboot
DeployCommand_Update -> deployUpdate "."
DeployCommand_Test (platform, extraArgs) -> deployMobile platform extraArgs
ObCommand_Run interpretPathsList certDir servePort -> withInterpretPaths interpretPathsList (run certDir servePort)
Expand Down
16 changes: 10 additions & 6 deletions lib/command/src/Obelisk/Command/Deploy.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@
{-# LANGUAGE FlexibleContexts #-}
{-# LANGUAGE LambdaCase #-}
{-# LANGUAGE OverloadedStrings #-}
{-# LANGUAGE TemplateHaskell #-}
{-# LANGUAGE PackageImports #-}
{-# LANGUAGE QuasiQuotes #-}
{-# LANGUAGE ScopedTypeVariables #-}
{-# LANGUAGE PackageImports #-}
{-# LANGUAGE TemplateHaskell #-}
{-# LANGUAGE ViewPatterns #-}
{- ORMOLU_DISABLE -}
{-|
Description:
Implementation of the CLI deploy commands. Deployment is done by intializing
Expand Down Expand Up @@ -161,8 +162,9 @@ deployPush
-- ^ Path to the staging directory
-> [String]
-- ^ nix builders arg string for the nix-build that builds the deployment artefacts
-> Bool
-> m ()
deployPush deployPath builders = do
deployPush deployPath builders reboot = do
hosts <- Set.fromList . filter (/= mempty) . lines <$> readDeployConfig deployPath "backend_hosts"
adminEmail <- readDeployConfig deployPath "admin_email"
enableHttps <- read <$> readDeployConfig deployPath "enable_https"
Expand Down Expand Up @@ -225,7 +227,7 @@ deployPush deployPath builders = do
[ "root@" <> host
, unwords
[ "bash -c"
, bashEscape (deployActivationScript outputPath)
, bashEscape (deployActivationScript outputPath reboot)
]
]
isClean <- checkGitCleanStatus deployPath True
Expand All @@ -247,8 +249,9 @@ deployPush deployPath builders = do
deployActivationScript
:: String
-- ^ The out path of the configuration to activate
-> Bool
-> String
deployActivationScript outPath =
deployActivationScript outPath reboot =
-- Note that we don't want to $(staticWhich "nix-env") here, because this is executing on a remote machine
-- This logic follows the nixos auto-upgrade module as of writing.
-- If the workflow is added to switch-to-configuration proper, we can simplify this:
Expand All @@ -258,7 +261,8 @@ nix-env -p /nix/var/nix/profiles/system --set "${bashEscape outPath}"
/nix/var/nix/profiles/system/bin/switch-to-configuration boot
booted="$(readlink /run/booted-system/{initrd,kernel,kernel-modules})"
built="$(readlink /nix/var/nix/profiles/system/{initrd,kernel,kernel-modules})"
if [ "$booted" = "$built" ]; then
echo "${bashEscape (show reboot)}"
if [[ "$booted" = "$built" && "${bashEscape (show reboot)}" == "${bashEscape (show False)}" ]]; then
Comment on lines +264 to +265
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems to me like it might be better to just template in the reboot if the flag is set rather than doing logic in bash

/nix/var/nix/profiles/system/bin/switch-to-configuration switch
else
/run/current-system/sw/bin/shutdown -r +1
Expand Down