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

Add fallback ref & preference rules for duplicate deployment targets #333

Merged
merged 2 commits into from
Oct 20, 2023

Conversation

bsquizz
Copy link
Collaborator

@bsquizz bsquizz commented Oct 11, 2023

The new containerized frontend apps have two deployment targets configured in the same environment -- a 'stable' target and a 'beta' target. Because of this, bonfire doesn't know which one to select when a user indicates something like --ref-env=insights-stage -- the deployment config from insights-stage that it selects to use for the git ref/image tag to deploy can be unpredictable.

Also, when bonfire is run with no --ref-env, it defaults to pulling from 'main/master' for all components. This isn't ideal for the frontend deployments which are pushing their betas into the main/master branch.

This PR:

  • Sets the default --ref-env to be 'insights-stage' (can be changed with an env var). For backends, this should have basically no impact, because nearly every stage deployment target is using master/main already anyways.
  • Adds a '--fallback-ref-env' argument. If we can't find a component's deployment config in the ref env, we will attempt to find it in the fallback ref env (by default this is 'insights-stage', but can be changed with an env var). If both the ref env and the fallback are the same, we don't bother analyzing the fallback env.
  • Adds a '--prefer' argument that allows users to customize how the bonfire "duplicate deploy target" preference logic works. For example, you can use bonfire deploy host-inventory --frontends=true --ref-env=insights-stage --prefer ENV_NAME=frontends-beta to prefer beta deployment targets. You can use --prefer multiple times if for some reason you wish to specify multiple parameters.
  • Changes the logic of _check_replace_other to use a number weighted system, and takes the --prefer input into account. Also, I noticed the 'replicas' logic needed a tweak, went ahead and updated it here.
  • Sets the default for --prefer to be ENV_NAME=frontends -- which would prefer consoledot stable frontend deployments.
  • Enhances the logging related to how the app/component list is built
  • Fixes a bug I found related to how we match namespaces to an environment. Some namespaces have the same name across environments or clusters. So we should match on the full namespace path in app-interface instead of just the name.

@bsquizz bsquizz force-pushed the prefer_stable_frontend_target branch from be4e444 to 6311aed Compare October 12, 2023 17:58
@bsquizz bsquizz changed the title Update preference rules for duplicate deployment targets Support custom preference rules for duplicate deployment targets Oct 12, 2023
@bsquizz bsquizz force-pushed the prefer_stable_frontend_target branch 2 times, most recently from aa6e75e to b5d3a49 Compare October 13, 2023 13:58
Copy link
Collaborator

@adamrdrew adamrdrew left a comment

Choose a reason for hiding this comment

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

LGTM! I was expecting this patch to be more intense but it was really less invasive than I pictured. I guess in the end this was just taking what was already there and just adding a new way to narrow down the querying. So, LGTM!

@bsquizz bsquizz changed the title Support custom preference rules for duplicate deployment targets Add fallback ref & preference rules for duplicate deployment targets Oct 20, 2023
@bsquizz
Copy link
Collaborator Author

bsquizz commented Oct 20, 2023

Tested this to ensure there's no impact to the backend configs. Here's a little script I'm using to ensure that every resource between 2 bonfire process commands is equivalent:

# compare.py <old file> <new file>

import json
import sys


with open(sys.argv[1]) as fp:
    old = json.loads(fp.read())

with open(sys.argv[2]) as fp:
    new = json.loads(fp.read())

for old_item in old["items"]:
    kind = old_item["kind"]
    name = old_item["metadata"]["name"]

    for new_item in new["items"]:
        if new_item["kind"] == kind and new_item["metadata"]["name"] == name:
            if old_item != new_item:
                print(f"diff found: {kind} {name}")

I am switching between bonfire's 'master' branch, for ex:

git checkout master
bonfire process patchman --frontends=true --clowd-env=test > old-patchman
git checkout prefer_stable_frontend_target
bonfire process patchman --frontends=true --clowd-env=test > new-patchman
python compare.py old-patchman new-patchman

Here are results for advisor:

❯ python compare.py old-advisor new-advisor
diff found: Frontend chrome
diff found: Frontend rbac
diff found: Frontend landing
diff found: Frontend dashboard
diff found: Frontend inventory
diff found: Frontend advisor
diff found: Secret sources-api-secrets

(the secret is different every time because the template is generating a random pre-shared key)

For patchman:

❯ python compare.py old-patchman new-patchman
diff found: Frontend patch
diff found: Frontend dashboard
diff found: Frontend rbac
diff found: Frontend chrome
diff found: Frontend inventory
diff found: Frontend landing

(the frontends are all different because now we're selecting the stage-stable frontends)

When using --ref-env=insights-production:

❯ python compare.py old-advisor new-advisor
diff found: Secret sources-api-secrets
❯ python compare.py old-patchman new-patchman
diff found: Frontend patch

(the patch frontend is different because we're falling back to select the stage-stable frontend... previously bonfire would have selected the stage-beta frontend)

Copy link
Collaborator

@adamrdrew adamrdrew left a comment

Choose a reason for hiding this comment

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

LGTM - I don't see any problems or anything that should change. All makes sense.

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.

2 participants