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

Jump to bottom

add gitops get profiles command #1227

Merged
merged 5 commits into from Jan 19, 2022
Merged

add gitops get profiles command #1227

merged 5 commits into from Jan 19, 2022

Conversation

aclevername
Copy link
Contributor

Closes: #1108

What changed?
New command:

gitops --namespace test-namespace get profiles
NAME    DESCRIPTION     AVAILABLE_VERSIONS
podinfo Podinfo Helm chart for Kubernetes       6.0.0,6.0.1

To be able to see the available profiles

How did you test it?
Manually & acceptance test

Release notes
Add new gitops get profiles command for discovering available profiles

Documentation Changes
TODO

@aclevername aclevername changed the title add gitops get profiles command add gitops get profiles command Dec 14, 2021
@aclevername aclevername marked this pull request as ready for review December 15, 2021 10:37
@aclevername aclevername force-pushed the profiles-get branch 3 times, most recently from 3472e1e to a419151 Compare December 16, 2021 10:30
@aclevername aclevername mentioned this pull request Dec 21, 2021
Closed
@foot foot mentioned this pull request Jan 6, 2022
Merged
nikimanoledaki
nikimanoledaki reviewed Jan 13, 2022
View reviewed changes
pkg/profiles/get.go Outdated Show resolved Hide resolved
cmd/gitops/get/profiles/cmd.go
return err
}

return profiles.GetProfiles(context.Background(), profiles.GetOptions{
Copy link

Choose a reason for hiding this comment

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

While this works, it relies on having access to the cluster through the kubeconfig and bypasses the config repo, which should be the Gitops SoT. I guess this is one way but not the only way that we want to be able to get profiles (maybe we need a separate ticket)? cc @bigkevmcd

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gitops SoT.

what does this mean 🤔

I guess this is one way but not the only way that we want to be able to get profiles (maybe we need a separate ticket)? cc @bigkevmcd

👍

Copy link

Choose a reason for hiding this comment

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

basically I'm wondering whether it's better to list the profiles that are in the cluster through the local kubeconfig or through the HelmReleases in /.weave-gitops/clusters/<cluster name>/system, or both?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is listing available profiles, as in what you can install (the charts that are availble from the HelmRepository resource), not what you've actually got installed.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess there are three possible lists of Profiles...

  1. List of Profiles in a cluster directory in a Git repository
  2. List of available profiles (to be installed)
  3. List of installed profiles in the cluster pointed at by your current context

So, we need to figure out how to be able to query for these

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This story/PR is definitely just for 2., but I agree we will need 1. and 3. at some point

Copy link

Choose a reason for hiding this comment

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

Sounds good! @Himangini and I will link up on Monday to create some follow-up tickets.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess there are three possible lists of Profiles...

  1. List of Profiles in a cluster directory in a Git repository
  2. List of available profiles (to be installed)
  3. List of installed profiles in the cluster pointed at by your current context

So, we need to figure out how to be able to query for these

This should've been discussed in planning.

Copy link
Contributor

Choose a reason for hiding this comment

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

This story/PR is definitely just for 2., but I agree we will need 1. and 3. at some point

@aclevername can you pair with @bigkevmcd and come up with tickets for 1. and 3. with details of what's required, please. Please also link this PR for reference in the new ones.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#1318

pkg/profiles/get_test.go Outdated Show resolved Hide resolved
Skarlso
Skarlso approved these changes Jan 14, 2022
View reviewed changes
Copy link
Contributor

@Skarlso Skarlso left a comment

Choose a reason for hiding this comment

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

Does what it should 👍

nikimanoledaki
nikimanoledaki approved these changes Jan 18, 2022
View reviewed changes
Copy link

@nikimanoledaki nikimanoledaki left a comment •
edited

Choose a reason for hiding this comment

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

LGTM! (go.mod needs updating, hopefully that resolves the Snyk issue)

@aclevername aclevername mentioned this pull request Jan 19, 2022
Closed
@aclevername aclevername merged commit c9730c1 into main Jan 19, 2022
@ozamosi ozamosi deleted the profiles-get branch May 12, 2022 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Skarlso Skarlso Skarlso approved these changes

@nikimanoledaki nikimanoledaki nikimanoledaki approved these changes

@cPu1 cPu1 Awaiting requested review from cPu1

@Himangini Himangini Awaiting requested review from Himangini

@bigkevmcd bigkevmcd Awaiting requested review from bigkevmcd

Assignees
No one assigned
Labels
area/profiles-improvement type/enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Move pctl get command to gitops get profile/profiles command
5 participants
@aclevername @Skarlso @bigkevmcd @Himangini @nikimanoledaki