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 recommendation rule controller #427

Merged
merged 2 commits into from
Jul 26, 2022
Merged

Conversation

qmhu
Copy link
Member

@qmhu qmhu commented Jul 21, 2022

What type of PR is this?

Feature

What this PR does / why we need it:

Add recommendation rule controller

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

@github-actions
Copy link
Contributor

github-actions bot commented Jul 21, 2022

🎉 Successfully Build Images.

Overview: https://finops.coding.net/public-artifacts/gocrane/crane/packages

Image Pull Command
crane-agent:pr-427-c9a5f8e docker pull finops-docker.pkg.coding.net/gocrane/crane/crane-agent:pr-427-c9a5f8e
dashboard:pr-427-c9a5f8e docker pull finops-docker.pkg.coding.net/gocrane/crane/dashboard:pr-427-c9a5f8e
metric-adapter:pr-427-c9a5f8e docker pull finops-docker.pkg.coding.net/gocrane/crane/metric-adapter:pr-427-c9a5f8e
craned:pr-427-c9a5f8e docker pull finops-docker.pkg.coding.net/gocrane/crane/craned:pr-427-c9a5f8e

@@ -15,3 +15,9 @@ const (
AnalyticsUidLabel = "analysis.crane.io/analytics-uid"
AnalyticsTypeLabel = "analysis.crane.io/analytics-type"
)

const (
Copy link
Member

Choose a reason for hiding this comment

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

where we need those labels?

Copy link
Member Author

Choose a reason for hiding this comment

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

To know the recommendation from a RecommendationRule by label selector. for example kubectl crane use it to get recommendation result.

Copy link
Member

Choose a reason for hiding this comment

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

got it.

mission.Message = fmt.Sprintf("Failed to get identity, key %s. ", k)
return
} else {
recommendation := existingRecommendation
Copy link
Member

Choose a reason for hiding this comment

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

I will implement the logic of this part. Can directly delete no use code.

Copy link
Member Author

@qmhu qmhu Jul 23, 2022

Choose a reason for hiding this comment

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

It's convenience to keep it for refactor

if currMissions == nil {
// create recommendation missions for this round
for _, id := range identities {
currMissions = append(currMissions, analysisv1alph1.RecommendationMission{
Copy link
Member

Choose a reason for hiding this comment

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

one mission for one recommendation rule or one object reference and one recommender?

Copy link
Member Author

Choose a reason for hiding this comment

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

one mission for one recommendation CR.

Copy link
Member

Choose a reason for hiding this comment

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

ok, one mission for one object reference and one recommender.

@xieydd
Copy link
Member

xieydd commented Jul 25, 2022

/LGTM

@xieydd xieydd self-requested a review July 25, 2022 06:23
@qmhu qmhu merged commit 70e813a into gocrane:main Jul 26, 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.

2 participants