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

Migrate cloudProviderId #197

Merged
2 commits merged into from Dec 5, 2022
Merged

Migrate cloudProviderId #197

2 commits merged into from Dec 5, 2022

Conversation

ghost
Copy link

@ghost ghost commented Nov 30, 2022

What type of PR is this?
/kind bug

What this PR does / why we need it:
Migrate cloudProviderId from osc:// to aws:///
Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #196

Special notes for your reviewer:

Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

TODOs:

  • squashed commits
  • includes documentation
  • adds unit tests

@ghost ghost requested review from outscale-mdr and outscale-hmi December 2, 2022 09:48
@ghost
Copy link
Author

ghost commented Dec 5, 2022

@outscale-hmi @outscale-mdr Are you ok with latest changes ?

provider-id: aws:///'{{ ds.meta_data.instance_id }}'
provider-id: aws://eu-west-2a/'{{ ds.meta_data.instance_id }}'
Copy link
Contributor

Choose a reason for hiding this comment

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

"eu-west-2a", it should be retrieve from the metadata.
The following command will give you the subregion:

curl  http://169.254.169.254/latest/meta-data/placement/availability-zone

You should test if {{ ds.meta_data.placement.availability_zone }} gives you what you want

Copy link
Author

Choose a reason for hiding this comment

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

Ok done thanks.

@@ -129,7 +129,7 @@ spec:
name: "{{ ds.meta_data.local_hostname }}"
kubeletExtraArgs:
cloud-provider: external
provider-id: osc://'{{ ds.meta_data.instance_id }}'
provider-id: aws://${OSC_SUBREGION_NAME}/'{{ ds.meta_data.instance_id }}'
Copy link
Contributor

Choose a reason for hiding this comment

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

Same takes what the metadata server gives you

Copy link
Author

Choose a reason for hiding this comment

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

Ok i change it

Copy link
Author

Choose a reason for hiding this comment

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

@outscale-mdr CI passed and validation on cloudgouv passed

Comment on lines +134 to +141
retryPeriod := 30 * time.Second
leaseDuration := 80 * time.Second
renewDeadline := 20 * time.Second
k8sManager, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{
LeaseDuration: &leaseDuration,
RenewDeadline: &renewDeadline,
RetryPeriod: &retryPeriod,
})
Copy link
Contributor

Choose a reason for hiding this comment

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

What is it for ?

Copy link
Author

@ghost ghost Dec 5, 2022

Choose a reason for hiding this comment

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

@outscale-mdr
To be sure that cluster api be the master controller, not the test.

(Ps Testenv and e2etest has to be slave controller.
It has to be controller to communicate with cluster api object)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok thank you but now are you sure the controller will no be a master ?

Copy link
Author

Choose a reason for hiding this comment

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

I try several time with my runner and the controller was not the master.

@ghost ghost requested a review from outscale-mdr December 5, 2022 12:29
@ghost ghost merged commit dd8db37 into outscale:main Dec 5, 2022
@ghost ghost added the kind/bug Bug resolution label Dec 7, 2022
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Bug resolution
Development

Successfully merging this pull request may close these issues.

Migrate e2etest to an other cloudProviderId
2 participants