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

default to quay based observability resources #279

Conversation

pb82
Copy link
Contributor

@pb82 pb82 commented Mar 23, 2023

With the coming version of KFM, the observability configuration changes and the observability resources are now obtained from quay instead of Github. OBSERVABILITY_CONFIG_ACCESS_TOKEN is no longer used. Github based resources are still supported, but the access token has to be provided as part of a new config file.

Instead, I would suggest to just update the KAS Installer to the new quay location.

NOTE: this can only be merged, once bf2fc6cc711aee1a0c2a/kas-fleet-manager#1647 has landed. The KFM Git ref also needs to be updated one available.

@miguelsorianod
Copy link
Contributor

With the coming version of KFM, the observability configuration changes and the observability resources are now obtained from quay instead of Github. OBSERVABILITY_CONFIG_ACCESS_TOKEN is no longer used. Github based resources are still supported, but the access token has to be provided as part of a new config file.

Instead, I would suggest to just update the KAS Installer to the new quay location.

NOTE: this can only be merged, once bf2fc6cc711aee1a0c2a/kas-fleet-manager#1647 has landed. The KFM Git ref also needs to be updated one available.

Would the credentials to be able to get the quay image need to be set somehow in kas installer?

@pb82
Copy link
Contributor Author

pb82 commented Mar 23, 2023

@miguelsorianod No, the KFM creates the image pull secret in the Observability namespace to pull that image.

@miguelsorianod
Copy link
Contributor

miguelsorianod commented Mar 23, 2023

@miguelsorianod No, the KFM creates the image pull secret in the Observability namespace to pull that image.

How/Where does the KFM get the content for the image pull secret?

@pb82
Copy link
Contributor Author

pb82 commented Mar 23, 2023

it's the same dockerconfig.json that also gets added to the FSO namespace, KFM gets it from the service secret in Vault. We have added permissions to the resources repo to the robot account used in there.

@miguelsorianod
Copy link
Contributor

miguelsorianod commented Mar 23, 2023

it's the same dockerconfig.json that also gets added to the FSO namespace, KFM gets it from the service secret in Vault. We have added permissions to the resources repo to the robot account used in there.

And KAS Installer gets it from the service secret in Vault?
What you are talking are specifics of KFM Stage / Production environments, which might not be necessarily true for dev environments and for kas installer.

@MikeEdgar
Copy link
Contributor

And KAS Installer gets it from the service secret in Vault?

The pull secret is created by the installer in <KFM Namespace>/kas-fleet-manager-image-pull-secret based on a user-provided username/password.

@miguelsorianod
Copy link
Contributor

And KAS Installer gets it from the service secret in Vault?

The pull secret is created by the installer in <KFM Namespace>/kas-fleet-manager-image-pull-secret based on a user-provided username/password.

Then my original question still applies, would the credentials, which we now know are set by kas installer, be able to get the quay image from the quay repository that contains the observability config?
Do we need to update the documentation of kas installer to make clear that it is a requirement that those credentials must have access to a specific quay repository?

@MikeEdgar
Copy link
Contributor

A few additional notes for when bf2fc6cc711aee1a0c2a/kas-fleet-manager#1647 is merged:

  • update the KFM default commit
    KAS_FLEET_MANAGER_GIT_REF="${KAS_FLEET_MANAGER_GIT_REF:-"cb483d50818dff77c108908cf80d6454a5a0b1e3"}"
  • remove occurrences of OBSERVABILITY_CONFIG_ACCESS_TOKEN (several throughout the repo)

@@ -75,7 +75,6 @@ fi
cat <<EOF
{
"LAUNCH_KEY": "${USER}",
"GITHUB_TOKEN": "${OBSERVABILITY_CONFIG_ACCESS_TOKEN}",
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we know what GITHUB_TOKEN is used for? Are we sure it is only used for the part around observability config retrieval?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I understand this part of the installer code correctly, it was only printing the configuration summary. There are no other references to GITHUB_TOKEN inside the KAS Installer.

Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure of that. It is true that it prints to stdout but in kas-installer the output of the scripts is reused for other elements sometimes.
See https://github.com/bf2fc6cc711aee1a0c2a/kas-installer/blob/main/README.md#running-e2e-test-suite-experimental where it seems the output of that script is used as some kind of configuration

@pb82 pb82 force-pushed the observability-operator-v4.2.0-prep branch from 081df37 to 3e493cc Compare March 28, 2023 09:44
@pb82 pb82 changed the title WIP: default to quay based observability resources default to quay based observability resources Mar 28, 2023
@miguelsorianod
Copy link
Contributor

The changes look good to me. However, we will need to continue the remaining cleanup of kas-installer and the pipelines with @MikeEdgar , as we still see some remaining elements to be cleant.
Let's merge and tag already to fix the e2e pipelines failing after the KFM PR that had been merged.

@miguelsorianod miguelsorianod merged commit 7319238 into bf2fc6cc711aee1a0c2a:main Mar 28, 2023
Copy link
Contributor

@MikeEdgar MikeEdgar left a comment

Choose a reason for hiding this comment

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

LGTM :-)

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.

3 participants