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

[grafana] Added support for .spec.loadBalancerClass #2730

Closed
wants to merge 52 commits into from

Conversation

Sheikh-Abubaker
Copy link
Collaborator

@Sheikh-Abubaker Sheikh-Abubaker commented Oct 20, 2023

Issue #2717

@Sheikh-Abubaker Sheikh-Abubaker changed the title Added support for .spec.loadBalancerClass [grafana] Added support for .spec.loadBalancerClass Oct 20, 2023
Signed-off-by: Sheikh-Abubaker <[email protected]>
Copy link
Collaborator

@zanhsieh zanhsieh left a comment

Choose a reason for hiding this comment

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

@Sheikh-Abubaker
Thanks for the contribution. Can you bump the version in Chart.yaml and fixing the lint-test error please?

@Sheikh-Abubaker
Copy link
Collaborator Author

@Sheikh-Abubaker Thanks for the contribution. Can you bump the version in Chart.yaml and fixing the lint-test error please?

I did bumped the version from 6.61.1 to 6.61.2 but that doesn't seem to have any effect and also about the lint-test error can you please guide me how to fix that error

@zanhsieh
Copy link
Collaborator

zanhsieh commented Oct 22, 2023

I did bumped the version from 6.61.1 to 6.61.2 but that doesn't seem to have any effect and also about the lint-test error can you please guide me how to fix that error

@Sheikh-Abubaker
Doesn't the github tell you what is the error if you scroll down? Line 200 vs 211:

https://github.com/grafana/helm-charts/pull/2730/files

@Sheikh-Abubaker
Copy link
Collaborator Author

Sheikh-Abubaker commented Oct 22, 2023

@Sheikh-Abubaker Doesn't the github tell you what is the error if you scroll down?

https://github.com/grafana/helm-charts/pull/2730/files

It does and I know the error is showing up due to the reason "[key-duplicates] duplication of key "type" in mapping" but my query is that we need to specify there config for two types of services "ClusterIP" and "LoadBalancer" so how do I do it with using "type" key twice ?

I did researched about this but I am getting a bit confused there as I am really very new to Open source and doesn't know that much, but willing to learn

@Sheikh-Abubaker
Copy link
Collaborator Author

@Sheikh-Abubaker Doesn't the github tell you what is the error if you scroll down? Line 200 vs 211:

https://github.com/grafana/helm-charts/pull/2730/files

Hi @zanhsieh,

I have fixed the error please check if everything is upto the mark.

@Sheikh-Abubaker
Copy link
Collaborator Author

Hi @zanhsieh can you please look into this PR if this is upto to the mark, I'm not in any sort of hurry but have participated in Hacktoberfest and it's almost end of Hacktoberfest2023 and I've still got 2 more PRs to be merged in order to reach 4/4 PRs merged criteria, so just wanted to know.

Sheikh-Abubaker and others added 12 commits October 26, 2023 19:57
Signed-off-by: Mahdi <[email protected]>
Signed-off-by: Sheikh-Abubaker <[email protected]>
Signed-off-by: Mahdi <[email protected]>
Signed-off-by: Sheikh-Abubaker <[email protected]>
Signed-off-by: Mahdi <[email protected]>
Signed-off-by: Sheikh-Abubaker <[email protected]>
Signed-off-by: Mahdi <[email protected]>
Signed-off-by: Sheikh-Abubaker <[email protected]>
Signed-off-by: Sheikh-Abubaker <[email protected]>
Signed-off-by: Sheikh-Abubaker <[email protected]>
Signed-off-by: Sheikh-Abubaker <[email protected]>
)

* ci: update-helm-repo with Github Apps JWT

Signed-off-by: Olha Yevtushenko <[email protected]>

* ci: enhance update-helm-repo with Github Apps

Signed-off-by: Olha Yevtushenko <[email protected]>

---------

Signed-off-by: Olha Yevtushenko <[email protected]>
Signed-off-by: Sheikh-Abubaker <[email protected]>
verejoel and others added 25 commits October 26, 2023 21:06
Signed-off-by: verejoel <[email protected]>
Signed-off-by: Sheikh-Abubaker <[email protected]>
Signed-off-by: verejoel <[email protected]>
Signed-off-by: Sheikh-Abubaker <[email protected]>
Signed-off-by: verejoel <[email protected]>
Signed-off-by: Sheikh-Abubaker <[email protected]>
Signed-off-by: verejoel <[email protected]>
Signed-off-by: Sheikh-Abubaker <[email protected]>
Signed-off-by: verejoel <[email protected]>
Signed-off-by: Sheikh-Abubaker <[email protected]>
Signed-off-by: verejoel <[email protected]>
Signed-off-by: Sheikh-Abubaker <[email protected]>
Signed-off-by: Emil Kordahl <[email protected]>
Signed-off-by: Sheikh-Abubaker <[email protected]>
Signed-off-by: Marton Schneider <[email protected]>
Signed-off-by: Sheikh-Abubaker <[email protected]>
Signed-off-by: Mahdi <[email protected]>
Signed-off-by: Sheikh-Abubaker <[email protected]>
Signed-off-by: Mahdi <[email protected]>
Signed-off-by: Sheikh-Abubaker <[email protected]>
Signed-off-by: Mahdi <[email protected]>
Signed-off-by: Sheikh-Abubaker <[email protected]>
Signed-off-by: Mahdi <[email protected]>
Signed-off-by: Sheikh-Abubaker <[email protected]>
Signed-off-by: Sheikh-Abubaker <[email protected]>
Signed-off-by: Sheikh-Abubaker <[email protected]>
)

* ci: update-helm-repo with Github Apps JWT

Signed-off-by: Olha Yevtushenko <[email protected]>

* ci: enhance update-helm-repo with Github Apps

Signed-off-by: Olha Yevtushenko <[email protected]>

---------

Signed-off-by: Olha Yevtushenko <[email protected]>
Signed-off-by: Sheikh-Abubaker <[email protected]>
Signed-off-by: Sheikh-Abubaker <[email protected]>
Signed-off-by: Sheikh-Abubaker <[email protected]>
Signed-off-by: Sheikh-Abubaker <[email protected]>
@Sheikh-Abubaker
Copy link
Collaborator Author

Sheikh-Abubaker commented Oct 26, 2023

@zanhsieh, in order to keep this PR up to date with main branch I pushed some latest changes but due to some reason the lint test failed in this commit to fix the lint error I committed again but this time using GUI and I think that was the mistake I made from that point no matter how many time ran rebase the DCO check kept failing so I think I should close this PR and open a fresh one

@Sheikh-Abubaker
Copy link
Collaborator Author

@zanhsieh, please checkout this new PR

Copy link
Collaborator

@zanhsieh zanhsieh left a comment

Choose a reason for hiding this comment

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

@Sheikh-Abubaker
Copy link
Collaborator Author

@Sheikh-Abubaker Can you sign DCO please?

https://github.com/grafana/helm-charts/pull/2730/checks?check_run_id=18089636060

@zanhsieh, Please check this new PR, I have fixed errors and also signed the DCO it's all done

@zanhsieh zanhsieh closed this Oct 28, 2023
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.

9 participants