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

providers: add scaleway #1794

Merged
merged 2 commits into from
Feb 7, 2024
Merged

providers: add scaleway #1794

merged 2 commits into from
Feb 7, 2024

Conversation

tormath1
Copy link
Contributor

Hi,

In this PR we add the support for Scaleway cloud provider. Locally tested on Flatcar + Scaleway:

./ignition --platform scaleway --stage fetch --config-cache ./config.json

While it's a regular IMDS endpoint, it has a small difference with other providers:

$ curl http://169.254.42.42/user_data/cloud-init
# For security reason, you need to query user data with a source port below 1024. For instance, with curl, run as root: # curl http://169.254.42.42/user_data/... --local-port 1-1023

I decided to go with a new FetchOption called LocalPort and to override the default DialContext.

Related to: coreos/fedora-coreos-tracker#866

When establishing a network connection random port selection from the
Kernel for local address is enough in 99.9% of the case.
For the 0.1%, let's add a way to customize the local port used.

Signed-off-by: Mathieu Tortuyaux <mtortuyaux@microsoft.com>
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Just a small nit, would it be possible to describe how scale way is fetching the data in a comment above. similar to how it was done here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comment added. ✔️

Copy link
Collaborator

@prestist prestist left a comment

Choose a reason for hiding this comment

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

Thank you for putting this together. Just a super small nit, but otherwise LGTM!

Signed-off-by: Mathieu Tortuyaux <mtortuyaux@microsoft.com>
Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

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

Some comments, but LGTM as is too. We can always tweak those things in a follow-ups.

Thanks for the patch!

internal/resource/url.go Show resolved Hide resolved
internal/resource/url.go Show resolved Hide resolved
@jlebon
Copy link
Member

jlebon commented Feb 7, 2024

Right gotcha. I misread the code here and thought we were retrying the same port in a loop. But we're clearly generating a new port number each time.

@jlebon jlebon merged commit c700fcf into coreos:main Feb 7, 2024
9 checks passed
@tormath1 tormath1 deleted the tormath1/scaleway branch February 7, 2024 15:45
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