Commit 4852e51d authored by AdamSimpson's avatar AdamSimpson
Browse files

Ensure builders aren't deleted multiple times and more importantly that...

Ensure builders aren't deleted multiple times and more importantly that reservations aren't accessed after they're removed.
parent b4e6bff4
Loading
Loading
Loading
Loading
Loading
+23 −8
Original line number Diff line number Diff line
@@ -9,9 +9,11 @@
namespace asio = boost::asio;

enum class ReservationStatus {
    pending,
    active,
    complete
    pending,             // Waiting on a builder to be provided by the queue
    active,              // Provided a builder
    request_complete,    // The queue request is complete and it's safe to cleanup any resources associated with reservation
    cleanup,             // post request complete cleanup has begun and the resources allocated to it are being cleaned up
    finalized            // post complete cleanup is complete and the reservation can be destroyed
};

// Reservations are handled by the queue and assigned builders as available
@@ -21,7 +23,7 @@ public:
                                                         ready_timer(io_service) {}

    ~Reservation() {
        status = ReservationStatus::complete;
        status = ReservationStatus::request_complete;
    }

    // Create an infinite timer that will be cancelled by the queue when the job is ready
@@ -34,16 +36,29 @@ public:
        return status == ReservationStatus::pending;
    }

    bool complete() const {
        return status == ReservationStatus::complete;
    bool request_complete() const {
        return status == ReservationStatus::request_complete;
    }

    bool active() const {
        return status == ReservationStatus::active;
    }

    void set_complete() {
        status = ReservationStatus::complete;

    bool finalized() const {
        return status == ReservationStatus::finalized;
    }

    void set_request_complete() {
        status = ReservationStatus::request_complete;
    }

    void set_enter_cleanup() {
        status = ReservationStatus::cleanup;
    }

    void set_finalize() {
        status = ReservationStatus::finalized;
    }

    boost::optional<Builder> builder;
+1 −1
Original line number Diff line number Diff line
@@ -12,7 +12,7 @@ public:
                                               reservation(queue.enter()){}

    ~ReservationRequest() {
        reservation.set_complete();
        reservation.set_request_complete();
    }

    Builder async_wait(asio::yield_context yield);
+8 −7
Original line number Diff line number Diff line
@@ -18,20 +18,21 @@ void BuilderQueue::tick(asio::yield_context yield) {
        if (reservation.builder) {
            unavailable_builders.insert(reservation.builder.get());

            // If the reservation is complete delete the builder
            if (reservation.complete()) {
            // If the reservation has completed delete the builder
            if (reservation.request_complete()) {
                reservation.set_enter_cleanup();
                asio::spawn(io_service,
                            [&](asio::yield_context destroy_yield) {
                            [this, &reservation](asio::yield_context destroy_yield) {
                                OpenStackBuilder::destroy(reservation.builder.get(), io_service, destroy_yield);
                                reservation.builder = boost::none;
                                reservation.set_finalize();
                            });
            }
        }
    }

    // Delete reservations that are completed and don't have an active builder associated with them
    // Delete reservations that are completed
    reservations.remove_if([](const auto &reservation) {
        return (reservation.complete() && !reservation.builder);
        return reservation.finalized();
    });

    // Available_builders = all_builders - unavailable_builders
@@ -56,7 +57,7 @@ void BuilderQueue::tick(asio::yield_context yield) {
    for (int i=0; i < request_count; i++) {
        pending_requests++;
        asio::spawn(io_service,
                    [&](asio::yield_context request_yield) {
                    [this](asio::yield_context request_yield) {
                        OpenStackBuilder::request_create(io_service, request_yield);
                        pending_requests--;
                    });
+1 −0
Original line number Diff line number Diff line
@@ -127,6 +127,7 @@ namespace OpenStackBuilder {
        destroy_child.wait();
        int exit_code = destroy_child.exit_code();

        // TODO an error "can't" occur, if an error deleting is detected it should make all attempts to fix it
        if (exit_code != 0) {
            logger::write("Builder with ID " + builder.id + " failed to be destroyed");
        }