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

added auto_upgrade flag #956

Merged
merged 1 commit into from
Jun 27, 2022
Merged

Conversation

Cl0udius
Copy link

Adds the possibility to set an auto_upgrade flag (true|false) to optionally prevent automatic AWX instance upgrades when the operator gets upgraded.

AWX will be upgraded if a deployment exists and the flag is set to true or no deployment exits.
With this we can set instances to "auto_upgrade: false" and recreate the deployment of multiple AWX instances step by step instead of doing it all at once.

Related issue: #936

@Cl0udius Cl0udius changed the title added auto_update flag added auto_upgrade flag Jun 21, 2022
namespace: "{{ ansible_operator_meta.namespace }}"
register: tower_deployment

- block:
Copy link
Member

Choose a reason for hiding this comment

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

instead of this block, might be best to split this out to another task. That is, move everything that is in your block into a install.yml and have the Check for presence of Deployment still in main.yml. then include and run the install.yml task if your auto_update condition is true.

Copy link
Author

Choose a reason for hiding this comment

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

moved to install.yml

@fosterseth
Copy link
Member

fosterseth commented Jun 21, 2022

@shanemcd @rooftopcellist how do we feel about this auto_upgrade boolean? I definitely see the use case. That is, not bumping the awx pods and leading to application downtime only because the operator changed.

auto_upgrade might not be the best term, maybe auto_redeploy? If the user hard-codes the AWX version to use, they aren't really upgrading per-se, rather just bumping the awx pods.

@shanemcd
Copy link
Member

I think I'm fine with both the idea and the naming of the variable. Even if you hardcode the version of the image, other stuff in the yaml might have changed too.

@rooftopcellist
Copy link
Member

@Cl0udius With this PR, if you set auto_upgrade to false, what exactly would the "manual upgrade" steps be? Could we add that to the README.md?

My guess is that for each instance, you would need to modify the AWX spec to:

  1. Temporarily change auto_upgrade to true, which will trigger the reconciliation loop for just that instance.
  2. Allow the reconciliation loop to complete, which will run the install.yml, and update the deployment yaml to point at the new AWX image.
  3. Containers will cycle picking up the new AWX version.
  4. Change auto_upgrade back to true on the spec. The reconciliation loop should one one more time, but will be a no-op.

^^ Is that what you had in mind?

@Cl0udius
Copy link
Author

@Cl0udius With this PR, if you set auto_upgrade to false, what exactly would the "manual upgrade" steps be? Could we add that to the README.md?

My guess is that for each instance, you would need to modify the AWX spec to:

1. Temporarily change auto_upgrade to `true`, which will trigger the reconciliation loop for just that instance.

2. Allow the reconciliation loop to complete, which will run the install.yml, and update the deployment yaml to point at the new AWX image.

3. Containers will cycle picking up the new AWX version.

4. Change auto_upgrade back to `true` on the spec.  The reconciliation loop should one one more time, but will be a no-op.

^^ Is that what you had in mind?

Hi,

that would be one way. Another thing we could do is to leave the flag on value false and just remove the deployment from K8S which will also trigger the loop and will rollout the new version.

I can add both ways if this would help.

BR

@Cl0udius Cl0udius force-pushed the add_auto_upgrade_parameter branch from b20eb22 to aa311b1 Compare June 22, 2022 09:47
README.md Outdated
With this parameter you can influence the behaviour during an operator upgrade.
If set to true the operator will upgrade the specific instance directly.
When the value is set to false and we have a running deployment the operator will not update the AWX instance.
This can be usefull when you have multiple AWX instances which you want to upgrade step by step instead all at once.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
This can be usefull when you have multiple AWX instances which you want to upgrade step by step instead all at once.
This can be useful when you have multiple AWX instances which you want to upgrade step by step instead all at once.
Suggested change
This can be usefull when you have multiple AWX instances which you want to upgrade step by step instead all at once.
This can be usefull when you have multiple AWX instances which you want to upgrade step by step instead all at once.

README.md Outdated
@@ -1034,6 +1036,23 @@ Example configuration of `no_log` parameter
no_log: 'true'
```

#### Auto upgrade
With this parameter you can influence the behaviour during an operator upgrade.
If set to true the operator will upgrade the specific instance directly.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
If set to true the operator will upgrade the specific instance directly.
If set to `true`, the operator will upgrade the specific instance directly.
Suggested change
If set to true the operator will upgrade the specific instance directly.
If set to true the operator will upgrade the specific instance directly.

README.md Outdated
#### Auto upgrade
With this parameter you can influence the behaviour during an operator upgrade.
If set to true the operator will upgrade the specific instance directly.
When the value is set to false and we have a running deployment the operator will not update the AWX instance.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
When the value is set to false and we have a running deployment the operator will not update the AWX instance.
When the value is set to `false` and we have a running deployment, the operator will not update the AWX instance.

README.md Outdated
With this parameter you can influence the behaviour during an operator upgrade.
If set to true the operator will upgrade the specific instance directly.
When the value is set to false and we have a running deployment the operator will not update the AWX instance.
This can be usefull when you have multiple AWX instances which you want to upgrade step by step instead all at once.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
This can be usefull when you have multiple AWX instances which you want to upgrade step by step instead all at once.
This can be usefull when you have multiple AWX instances which you want to upgrade step by step instead of all at once.

# Just execute deployment steps when auto_upgrade is true or when no deployment exists
- name: Start installation
include_tasks: install.yml
when: (tower_deployment['resources'] | length > 0 and auto_upgrade == true) or (tower_deployment['resources'] | length == 0)
Copy link
Member

Choose a reason for hiding this comment

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

You probably need to cast this to a boolean:

Suggested change
when: (tower_deployment['resources'] | length > 0 and auto_upgrade == true) or (tower_deployment['resources'] | length == 0)
when: (tower_deployment['resources'] | length > 0 and auto_upgrade | bool) or (tower_deployment['resources'] | length == 0)

README.md Outdated

```yaml
spec:
auto_upgrade: 'true'
Copy link
Member

Choose a reason for hiding this comment

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

Is this intentionally represented as a string here?

Copy link
Author

@Cl0udius Cl0udius Jun 23, 2022

Choose a reason for hiding this comment

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

Nope. Changed now.

README.md Outdated
@@ -1095,6 +1114,25 @@ Then install the new AWX Operator by following the instructions in [Basic Instal

Once the new AWX Operator is up and running, your AWX deployment will also be upgraded.

##### Upgrade of instances with auto upgrade
Copy link
Member

Choose a reason for hiding this comment

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

I would move this up under the #### Auto upgrade section.

Copy link
Author

Choose a reason for hiding this comment

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

Moved like suggested

@Cl0udius Cl0udius force-pushed the add_auto_upgrade_parameter branch from aa311b1 to 35d4954 Compare June 23, 2022 13:05
Copy link
Member

@shanemcd shanemcd left a comment

Choose a reason for hiding this comment

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

I think people will like this feature. Thank you! ⭐

@shanemcd shanemcd merged commit 34b6354 into ansible:devel Jun 27, 2022
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.

5 participants